rcantin-w · Sep 16, 2026 · Sep 15, 2026 · Sep 16, 2026 · Sep 16, 2026
@@ -209,12 +209,12 @@ Your Catalogue API types (`Work`, `WorkBasic`, `Item`, etc.) are manually define
 
 ## Acceptance Criteria
 
-- [ ] Fixed both `setSearchResults` implicit `any` types
-- [ ] (Optional) Added `ImageService` type to `content/webapp/types/item-viewer.ts`
-- [ ] Documented existing type structure for reference
-- [ ] Checked for duplicate type definitions across files
-- [ ] `yarn tsc` runs without errors in ItemViewer files
-- [ ] All team members understand what types exist and where they live
+- [x] Fixed both `setSearchResults` implicit `any` types - now `(v: SearchResults | null) => void` in both context files
+- [x] (Optional) Added `ImageService` type to `content/webapp/types/item-viewer.ts` - used as `PartialImageService = Pick<ImageService, '@id'>`
+- [x] Documented existing type structure for reference
+- [x] Checked for duplicate type definitions across files
+- [x] `yarn tsc` runs without errors in ItemViewer files
+- [ ] All team members understand what types exist and where they live - not something the code can evidence, left for the team to judge
 
 ## What This Phase Achieves
 

@@ -125,15 +125,17 @@ import ItemViewerContextV2 from '@weco/common/contexts/ItemViewerContextV2';
 
 ## Success Criteria
 
-- [ ] Feature flag defined in `toggles.ts`
-- [ ] ItemViewerContextV2 created (identical to ItemViewerContext)
-- [ ] All original components renamed to `.legacy.tsx`
-- [ ] Wrapper component loads legacy or refactored based on flag
-- [ ] All `.refactored.tsx` files created and import ItemViewerContextV2
-- [ ] Application runs with flag OFF (uses legacy)
-- [ ] Application runs with flag ON (uses refactored, identical behaviour)
-- [ ] No TypeScript errors
-- [ ] No console warnings
+Met, though several were delivered under different names than planned - see the naming note in the [README](./README.md).
+
+- [x] Feature flag defined in `toggles.ts` - as `itemViewerRefactor`
+- [x] ItemViewerContextV2 created (identical to ItemViewerContext) - built as `contexts/ItemViewerContext/refactored.tsx`, with `legacy.tsx` beside it
+- [x] All original components renamed to `.legacy.tsx` - moved into a `legacy/` directory instead of renamed
+- [x] Wrapper component loads legacy or refactored based on flag - `IIIFViewer/index.tsx`
+- [x] All `.refactored.tsx` files created and import ItemViewerContextV2 - created under `refactored/`
+- [x] Application runs with flag OFF (uses legacy)
+- [x] Application runs with flag ON (uses refactored, identical behaviour)
+- [x] No TypeScript errors
+- [ ] No console warnings - a temporary log reporting which context is in use was added deliberately, in the context barrel. Removing it is item 1 of [#13273](https://github.com/wellcomecollection/wellcomecollection.org/issues/13273), and has to happen before the toggle goes on publicly.
 
 ## Next Steps
 

@@ -220,12 +220,12 @@ After this phase, consider splitting other components with multiple modes:
 
 ## Success Criteria
 
-- [ ] `MainViewer` is a simple router (<20 lines)
-- [ ] `VirtualizedImageViewer` handles only image-only works
-- [ ] `PaginatedItemViewer` handles only mixed/archive works
-- [ ] All tests pass
-- [ ] No behavioral changes detected
-- [ ] Code is easier to understand and test
+- [x] `MainViewer` is a simple router (<20 lines) - it routes and does nothing else, but the file is 32 lines: about 14 of routing, the rest imports and a styled container
+- [x] `VirtualizedImageViewer` handles only image-only works
+- [x] `PaginatedItemViewer` handles only mixed/archive works
+- [x] All tests pass
+- [x] No behavioral changes detected
+- [x] Code is easier to understand and test
 
 ## Time Estimate
 

@@ -132,7 +132,7 @@ All tests should pass.
 
 ### 1.6 (Optional) Manual Testing
 
-See [13-testing-strategy.md](./13-testing-strategy.md) for comprehensive manual testing checklist.
+See [14-testing-strategy.md](./14-testing-strategy.md) for comprehensive manual testing checklist.
 
 **Quick smoke tests:**
 - [ ] Viewer loads correctly
@@ -158,10 +158,10 @@ Self-documenting code! No mental gymnastics required.
 
 ## Success Criteria
 
-- [ ] All automated tests pass (context + components)
-- [ ] TypeScript compiles with no errors
-- [ ] Components use context values (not calculating locally)
-- [ ] Feature flag toggle works (old vs new identical behaviour)
+- [x] All automated tests pass (context + components)
+- [x] TypeScript compiles with no errors
+- [x] Components use context values (not calculating locally) - with two deliberate exceptions under principle 7: `canNavigateNext`/`canNavigatePrevious` stay local to `ViewerBottomBar`, their only consumer, derived from the shared `totalCanvases`; and `hasIiifImageService` stays local to `IIIFViewer`. `currentCanvasIndex` was never added at all - see the [README](./README.md).
+- [x] Feature flag toggle works (old vs new identical behaviour)
 - [ ] Manual testing checklist complete (optional)
 
 ## Time Breakdown

@@ -154,10 +154,10 @@ All should pass.
 
 ## Success Criteria
 
-- [ ] Tests for `useDownloadOptions` pass
-- [ ] `ViewerTopBar` ~65 lines shorter
-- [ ] Download dropdown still works identically
-- [ ] All download types still appear correctly
+- [x] Tests for `useDownloadOptions` pass
+- [x] `ViewerTopBar` ~65 lines shorter
+- [x] Download dropdown still works identically
+- [x] All download types still appear correctly
 
 ## Time Breakdown
 

@@ -69,9 +69,9 @@ const { isCurrentCanvasRestricted } = useItemViewerContextV2();
 
 ## Success Criteria
 
-- [ ] `isCurrentCanvasRestricted` in context
-- [ ] `ViewerTopBar` uses context value
-- [ ] Restricted badge appears correctly
+- [x] `isCurrentCanvasRestricted` in context
+- [x] `ViewerTopBar` uses context value
+- [ ] Restricted badge appears correctly - there is no restricted badge, in either tree. What `isCurrentCanvasRestricted` actually gates in `ViewerTopBar` is hiding the download button on a restricted canvas, unless the user is staff with restricted access.
 
 ## Time: ~1 hour
 

@@ -5,77 +5,42 @@
 **Effort:** 30 minutes  
 **Risk:** Low  
 **Priority:** Medium  
+**Status:** Done - [#12989](https://github.com/wellcomecollection/wellcomecollection.org/issues/12989)  
 **Previous:** [Phase 5: Restriction Status](./11-phase-5-restriction-status.md)  
 **Next:** [Phase 7: Cleanup](./13-phase-7-cleanup.md)  
 
 ## Goal
 
-Since `currentCanvasIndex` is now in context (from Phase 1), remove all duplicate `queryParamToArrayIndex(query.canvas)` calls across components.
+Remove duplicated calculations of "which canvas is currently showing" across components.
 
-## Why?
+## What this phase originally assumed
 
-**Before:**
-```typescript
-// Thumbnails.tsx
-const index = queryParamToArrayIndex(query.canvas);
+As planned, this was a find-and-replace: since `currentCanvasIndex` was to be on the context, swap every `queryParamToArrayIndex(query.canvas)` call for it, in `Thumbnails.tsx`, `NoScriptImage.tsx` and `MultipleManifestList.tsx`.
 
-// NoScriptImage.tsx
-const index = queryParamToArrayIndex(query.canvas);
+Neither half of that held by the time the phase came round:
 
-// MultipleManifestList.tsx
-const index = queryParamToArrayIndex(query.canvas);
-```
+- `currentCanvasIndex` was deliberately never added to the context. It would duplicate `query.canvas`, which is already there, and the 1-based canvas number and the 0-based array index are kept as distinct concepts throughout. See the note in [README](./README.md).
+- None of the three named files calculate a canvas index any more. `Thumbnails.tsx` calls `queryParamToArrayIndex` on `query.page` and `MultipleManifestList.tsx` on `query.manifest` - different query params, unrelated to the current canvas - and `NoScriptImage.tsx` doesn't call it at all, having moved to the shared `getCanvasesForPage` helper in Phase 3.
 
-**Problem:** Same calculation repeated in 3+ files.
+So the phase became an audit of the whole `refactored/` directory instead.
 
-**After:**
-```typescript
-const { currentCanvasIndex } = useItemViewerContextV2();
-```
+## What the audit found
 
-Single source of truth!
+One genuine duplicate, in `GridViewer.tsx`: the memoised `Cell` recalculated `queryParamToArrayIndex(query.canvas)` for its own `aria-current`, once per rendered grid cell, while the parent already calculated the identical value for its scroll-to-row effect. Fixed by calculating it once in `GridViewer` and passing it down through `itemData`, plus a first test for that component.
 
-## Files to Update
+Four lookalikes were checked and deliberately left alone, being different logic that happens to read similarly:
 
-Find all occurrences:
-
-```bash
-cd content/webapp
-grep -r "queryParamToArrayIndex(query.canvas)" .
-```
-
-### Expected files (update each):
-
-1. **`Thumbnails.tsx`** (lines 38, 49)
-2. **`NoScriptImage.tsx`** (line 42)
-3. **`MultipleManifestList.tsx`** (lines 23, 37)
-4. Any others found
-
-### Replace pattern:
-
-```typescript
-// OLD:
-const index = queryParamToArrayIndex(query.canvas);
-// or
-queryParamToArrayIndex(query.canvas)
-
-// NEW:
-const { currentCanvasIndex } = useItemViewerContextV2();
-// or just use currentCanvasIndex directly
-```
-
-## Testing
-
-- [ ] Thumbnails highlight correct canvas
-- [ ] NoScript image shows correct canvas
-- [ ] Multiple manifest list shows correct position
-- [ ] All components that use index still work
+- `GridViewer`'s `Cell` and `VirtualizedImageViewer`'s `ItemRenderer` each have a local `currentCanvas`, but those are per-cell and per-row canvases from iterating the whole list, not the one current canvas on the context.
+- `VirtualizedImageViewer` calls `hasRestrictedItem(canvases[0])`, which is about the first canvas specifically, for a margin adjustment - not the current one.
+- `IIIFViewer.tsx` reads `document.fullscreenElement` directly, to skip resize handling while a native video player is fullscreen. That's a different question from the fullscreen toggle state centralised in Phase 3.
 
 ## Success Criteria
 
-- [ ] No more `queryParamToArrayIndex(query.canvas)` calls in consuming components
-- [ ] All components use `currentCanvasIndex` from context
-- [ ] Test coverage maintained
+The criteria as originally written, with what actually happened against each:
+
+- [x] No more `queryParamToArrayIndex(query.canvas)` calls in consuming components - met, though by this point the only one left was the `GridViewer.tsx` duplicate above
+- [ ] All components use `currentCanvasIndex` from context - not done, and no longer wanted: the value was deliberately never added to the context
+- [x] Test coverage maintained - `GridViewer.test.tsx` added, the component's first test
 
 ## Time: ~30 minutes
 

@@ -4,6 +4,8 @@
 
 This checklist helps you track progress through each phase. Check off items as you complete them.
 
+Phases 0-6 are done; only Phase 7 is outstanding. The fine-grained boxes below are left as originally written rather than ticked retrospectively - each phase document's own success criteria have been checked off against the code instead, including the handful that weren't met as specified. Phase 6 and Phase 7 here have been rewritten, because their items no longer described the work to be done.
+
 ## Before Starting
 
 - [ ] Create feature branch: `refactor/iiif-viewer-context`
@@ -163,13 +165,13 @@ Duration: 1-1.5 hours
 
 ## Phase 6: Duplicate Index Calls
 
-- [ ] Find all `queryParamToArrayIndex(query.canvas)` calls
-- [ ] Update Thumbnails.tsx to use `currentCanvasIndex` from context
-- [ ] Update NoScriptImage.tsx to use `currentCanvasIndex`
-- [ ] Update MultipleManifestList.tsx to use `currentCanvasIndex`
-- [ ] Update any other files found
-- [ ] Test thumbnails highlight correctly
-- [ ] Test all navigation works
+This ran as an audit rather than the find-and-replace planned here: `currentCanvasIndex` was never added to the context, and none of the three files named below calculate a canvas index any more. See [Phase 6](./12-phase-6-duplicate-calls.md) for what the audit found.
+
+- [x] Find all `queryParamToArrayIndex(query.canvas)` calls
+- [x] Audit the whole `refactored/` directory for duplicated current-canvas calculations
+- [x] Deduplicate the one real case, in `GridViewer.tsx`
+- [x] Test thumbnails highlight correctly
+- [x] Test all navigation works
 
 **Time checkpoint:** Should take ~30 minutes
 
@@ -179,14 +181,14 @@ Duration: 1-1.5 hours
 
 **Only do this after toggle defaults to ON for 1+ week with no issues!**
 
-- [ ] Remove feature flag from `toggles.ts`
-- [ ] Delete all `.legacy.tsx` files- [ ] Rename `.refactored.tsx` to `.tsx`
-- [ ] Delete wrapper `index.tsx`
-- [ ] Delete old `ItemViewerContext` directory
-- [ ] Rename `ItemViewerContextV2` to `ItemViewerContext`
-- [ ] Update all imports from V2 to standard
-- [ ] Rename test files (remove .refactored)
-- [ ] Update test imports
+- [ ] Remove the `itemViewerRefactor` flag from `toggles/webapp/toggles.ts`, and redeploy the toggles package
+- [ ] Delete `IIIFViewer/legacy/` and `contexts/ItemViewerContext/legacy.tsx`
+- [ ] Delete the `IIIFViewer/index.tsx` switch and move `refactored/`'s contents up a level
+- [ ] Fold `contexts/ItemViewerContext/refactored.tsx` into `index.tsx`, dropping the flag-switching barrel
+- [ ] Remove `isRefactoredContext` and the migration console log
+- [ ] Put every consumer on one context import path
+- [ ] Collapse the dual-context test harness in `test/fixtures/iiif/render.tsx`
+- [ ] Drop the per-test `jest.mock` of `useFeatureFlags` and the `useRefactoredContext` arguments
 - [ ] Run all tests - still pass
 - [ ] `yarn tsc` - no errors
 - [ ] Application runs correctly