Proposal H, Step 1: /display route shell behind a feature flag - #87
Merged
WilfordGrimley merged 6 commits intoJul 18, 2026
Merged
Conversation
Survey + design + static HTML mockups for one page that replaces the "Choose Art" editor and the PDF export step with a live print-sheet preview (PagePreview/computeLayout, default 4x2 landscape) plus a persistent card-details rail carrying the existing per-slot instrument set (candidate picker, confirm affordance, attribute chips, requested- printing badge, bleed override, artist line, slot actions). Zero feature code — design doc, five breakpoint mockups, and their README only. Build not started; awaiting owner review.
Owner amendment: the rail's instruments are collapsible sections built from the existing AutofillCollapse component (same one PDFGenerator's settings groups already use), not a flat stack. Always-visible header (identity, requested-printing badge, Confirm? affordance) sits outside the accordion as status; Choose Image opens by default, Attributes/ Print Options/Artist/Slot Actions collapse by default. Updates the design doc's §2 (new amendment subsection + ASCII diagram), §4.2-4.4 (accordion-aware wording), and §5's component mapping table, plus all 5 mockups + shared.css to render real open/collapsed accordion sections instead of a flat instrument stack.
…oposal-h-step1-display-route-04bam2
New route (docs/proposals/proposal-h-unified-display-page.md's §6 migration plan, step 1), gated off by default via NEXT_PUBLIC_UNIFIED_DISPLAY_ENABLED: - Top toolbar: page N of M pagination, Fronts/Backs toggle (reuses the existing frontsVisible view setting), a small live subset of print settings (paper size, bleed edge, guides) driving the real computeLayout() the sheet renders from, and a working link to the classic editor's Print tab (full inline export is Step 3). - Live sheet: PagePreview/computeLayout reused as-is, paginated across the whole deck (not just page 1, unlike PDFGenerator's fast preview) via a new displayPagination.ts helper that preserves (face, slot) identity - CardSelectionModeToPaginator discards that, which this page's click-to-select needs. Only the current page's slots are resolved to thumbnail URLs; PagePreview gained optional onSlotClick/selectedSlotIndex props (unused by existing callers) and loading="lazy"/decoding="async" on its <img>. - Rail: always-visible status header (identity, requested-printing badge) + a 5-section AutofillCollapse accordion per the owner's amendment (Choose Image open by default, Attributes/Print Options/Artist/Slot Actions collapsed), each section a labeled stub naming which Step 2 PR fills it in. Moved the generic `chunk` helper from PDF.tsx to common/utils.ts (PDF.tsx now re-exports it for its existing callers) - importing anything from PDF.tsx pulls in @react-pdf/renderer's ESM-only bundle, which broke Jest for the new pure pagination helper's own test. Includes the Proposal H design doc + mockups (previously PR #84) per this task's instruction that they ride in with this step's flag. Tests: displayPagination.test.ts, featureFlags.test.ts, PagePreview's new click/selection/lazy-load cases, and tests/DisplayPage.spec.ts (a real Playwright suite exercising the flagged route end to end - empty state, live sheet + pagination, slot select, accordion defaults + reset-on-reselect, Fronts/Backs toggle, Guides toggle). Full existing Jest suite (315 tests) and a cross-section of PDF/editor Playwright specs both pass with zero regressions.
… one PR #87's "Formatting and static type checking" check failed - the pre-commit-pinned prettier@2.7.1 disagreed with whatever newer prettier version frontend/node_modules resolved for my earlier `npx prettier --write` pass (docs/lessons.md's "trust CI history, not a matching local venv" lesson, prettier edition). Installed pre-commit and ran its actual prettier hook against every affected file; content unchanged, only line-wrapping/formatting. Verified idempotent (a second run reports zero further changes) and re-ran tsc/eslint/jest (315/315) to confirm no corruption per docs/lessons.md's prettier@2.7.1 non-idempotency entry.
WilfordGrimley
pushed a commit
that referenced
this pull request
Jul 18, 2026
…ecks nav entry Mounts AuthWidget in the navbar (relocated off /whatsthat) and adds a "My Decks" top-level nav entry, gated on an authenticated whoami session, alongside it. The AuthWidget/Navbar/whatsthat changes are identical to PR #86 (claude/proposal-g-signin-navbar), carried forward for the same reason as the prior commit: PR4b needs sign-in visible everywhere for "My Decks" to be reachable, and #86 is still open. Deliberately did NOT carry over that branch's stale removal of the Display (beta) nav entry - that branch predates Proposal H (#87), which added Display; dropping it here would be a regression, not a carry-forward. /myDecks route itself doesn't exist yet - added in a following commit.
This was referenced Jul 18, 2026
Merged
WilfordGrimley
added a commit
that referenced
this pull request
Jul 18, 2026
* Proposal G, PR4b: carry forward frontend prerequisites from PR3/PR4a frontend/src/common/schema_types.ts (regenerated saved-deck types) and frontend/src/store/api.ts's Kind->VoteQueueRequestKind import fix come from PR #88 (claude/proposal-g-saved-decks-api); savedDeckCrypto.ts, its tests, and the jest.setup.ts crypto.subtle polyfill come from PR #89 (claude/proposal-g-crypto-module). Both PRs are still open, so this branch is based on master directly (not stacked) and carries identical copies of just the frontend files this UI work needs, to avoid a 3-deep PR stack. Verified byte-identical against each source branch; tsc --noEmit and the crypto module's jest suite both pass on this base. Deviation note for the merge-time checklist: once PR #88 and/or #89 merge to master, this branch's copies of these files will already match what lands there — rebase onto master before merging PR4b to drop the now-redundant duplicate commit cleanly (or let git no-op it; content is identical either way). * Proposal G, PR4b: carry forward sign-in relocation from PR2, add My Decks nav entry Mounts AuthWidget in the navbar (relocated off /whatsthat) and adds a "My Decks" top-level nav entry, gated on an authenticated whoami session, alongside it. The AuthWidget/Navbar/whatsthat changes are identical to PR #86 (claude/proposal-g-signin-navbar), carried forward for the same reason as the prior commit: PR4b needs sign-in visible everywhere for "My Decks" to be reachable, and #86 is still open. Deliberately did NOT carry over that branch's stale removal of the Display (beta) nav entry - that branch predates Proposal H (#87), which added Display; dropping it here would be a regression, not a carry-forward. /myDecks route itself doesn't exist yet - added in a following commit. * Proposal G, PR4b: deck payload/dirty-check plumbing + RTK Query wiring - features/savedDecks/deckPayload.ts: the plaintext shape encrypted wholesale (including its own name), serialize/parse helpers, and deviceLocal marking for LocalFile-sourced slots (identifiers are device-specific and meaningless elsewhere, so only the flag survives - the card grid's existing empty-slot UI becomes the honest re-pick placeholder). - store/slices/savedDeckSessionSlice.ts: tracks which saved deck (if any) the editor represents - session-only, deliberately not wired into listenerMiddleware's localStorage persistence. - projectSlice.loadProject / finishSettingsSlice.loadFinishSettings: atomic whole-project replacement, needed for loading a saved deck (no existing reducer does this - every other one merges into place). - features/savedDecks/selectors.ts: selectIsCurrentProjectDirty, per the frontend spec's exact definition (differs from last load/save, or is non-empty with no prior save at all). - store/api.ts: the 7 saved-deck/crypto-profile RTK Query endpoints (SavedDecks/CryptoProfile cache tags, credentials: "include" + CSRF header matching the existing moderation-write convention), with skip options so anonymous sessions never fire a doomed authenticated request. Own-caught fix while wiring the recovery UI: PR4a's recovery-flow test exercised changePassphrase but never reissued a recovery key, even though the ZK addendum's recovery flow explicitly re-wraps BOTH slots (passphrase under the new passphrase, recovery under a FRESH recovery key) once the old recovery key has actually been used - an ordinary passphrase change (already covered by a separate test) correctly leaves the recovery slot alone, but the full recovery path is a distinct case that wasn't covered in savedDeckCrypto.ts at all. Added rewrapMasterKeyWithNewRecoveryKey and extended the recovery-flow test to cover the new key end-to-end, including that the superseded old recovery key no longer unwraps the new slot. * Proposal G, PR4b: CryptoSessionProvider (in-memory master key context) A plain React Context (not Redux, since CryptoKey isn't serializable), mounted in Layout.tsx alongside ClientSearchContextProvider. Exposes status (anonymous/loading/no-profile/locked/unlocked), the unlocked master key, and createProfile/unlockWithPassphrase/ recoverAndSetNewPassphrase/lock - wired to the getCryptoProfile/ saveCryptoProfile endpoints added in the previous commit. The master key never persists anywhere, so it clears itself on every reload; lock() just does that sooner. 6 tests cover every status transition and the recovery flow's fresh recovery key end to end (createProfile, wrong/correct passphrase unlock, recover-and-reissue, lock), using a small harness component in the absence of any existing renderHook precedent in this codebase's test suite - matches the established render()+screen+MSW convention instead. * Proposal G, PR4b: passphrase creation + unlock modals - RecoveryKeyDisplay: the show-once recovery key step (download/print/copy + an explicit "I've saved this" acknowledgement gate before continuing), shared by both modals below since both flows end with a fresh recovery key to show. - PassphraseSetupModal: the first-save flow - passphrase + confirm, the verbatim-spirit unrecoverability warning, then RecoveryKeyDisplay. - UnlockModal: the once-per-session unlock prompt, with a "Forgot your passphrase?" branch into the recovery flow (paste recovery key + set a new passphrase -> reissues a fresh recovery key via recoverAndSetNewPassphrase -> RecoveryKeyDisplay again). Own-caught bug, found via a genuine test failure (not flakiness): status in cryptoSession.tsx fell through to "anonymous" whenever isAuthenticated was false - including the instant before the whoami query itself had even resolved. UnlockModal's tests failed with a misleading "wrong passphrase" error because they could submit before the crypto profile had loaded, since nothing signaled that loading state (masterKey != null ? "unlocked" : cryptoProfileQuery.data == null ? "loading" : ... never entered from the "anonymous" branch). Fixed by giving whoami's own in-flight state a distinct "loading" status ahead of the isAuthenticated check, added a regression test that delays the whoami response and asserts "loading" appears first, and added an isProfileLoading guard to UnlockModal (belt and suspenders: both the submit handler and the button's disabled state) so a real click during that window can never misfire either. * Proposal G, PR4b: My Decks page - deckPayload.ts: encryptDeckPayloadForSave (fresh per-save DEK; the server has no preference between create/update) and decryptSavedDeckSummary (unwrap DEK -> decrypt -> parse), the wire-format encrypt/decrypt pair the Save action and this page both need. - MyDecksPage: lists every saved deck, decrypted client-side once the crypto session is unlocked (prompting via UnlockModal automatically when locked). Named decks and snapshots render as separate groups. "Open in editor" loads the decrypted project/finishSettings into Redux and records the current-deck breadcrumb state, then navigates to /editor. Per-deck delete (confirm via window.confirm, matching the existing moderation-panel convention for destructive actions - no dedicated confirm-modal component exists in this codebase to reuse). A "Lock" action clears the in-memory master key. Account reset is reachable from both the locked AND unlocked states (getSavedDecks is fetched independently of decryption) since its entire purpose is recovering access when unlock is impossible - gated on an explicit second confirming click naming the exact deck count, not a modal. - Deviation: "Discord-gated" account reset is satisfied by requiring an already-authenticated session (the same as every other saved-deck action) rather than adding a fresh Discord re-auth redirect - the backend's post_reset_saved_decks has no freshness/recency check of its own to justify one, so a redirect step would be security theater without backend enforcement behind it. - /myDecks route (frontend/src/pages/myDecks.tsx), matching the nav entry already added. 6 tests cover every session state (anonymous, no-profile, locked-then- unlock, decrypted list grouping), the open-in-editor redux/navigation wiring, delete-with-confirmation, and the two-click reset gate. * Proposal G, PR4b: editor wiring - Save action, breadcrumb, load safety flow - SaveDeckModal: the explicit Save action (name prompt pre-filled from the current deck, local-file-slot warning, encrypts and calls saveDeck, records the returned key). Assumes the crypto session is already unlocked. - LoadSafetyModal: the loss-proof-by-construction load flow (frontend spec §4) - dirty + logged-in always saves a safety copy first, never skippable. Offers "Update {name}" vs "Save as new snapshot" when the current content is itself an already-saved deck; just an inline- renameable snapshot save (no skip option) when it was never saved. - SavedDeckPanel: the reverse breadcrumb ("Editing: {name}" / "Unsaved project") plus the Save button, rendered only when authenticated. Clicking Save runs PassphraseSetupModal or UnlockModal first if the crypto session isn't ready. Also raises the one-time anonymous->login adopt-by-save toast (informational only - the Toasts system has no action-button support, and extending shared toast infra for one caller wasn't worth it, so it just points at the Save button below). - Wired SavedDeckPanel into ProjectEditor's action cluster, and LoadSafetyModal into MyDecksPage's "Open in editor" (dirty-check via selectIsCurrentProjectDirty; empty/clean editors still load immediately, no prompt). 15 new tests across the three modals/panel, using a small status-exposing test harness to reliably wait for the crypto session to actually unlock before interacting (a bare "field is present" check isn't a real signal, since these components render their form regardless of lock state and just no-op an early submit). * Fix import-sort lint errors caught by a whole-project next lint pass selectors.ts and api.ts had passed every earlier per-file eslint/prettier check in this branch's individual commits, but a full-project `next lint` (not run until now) caught two real simple-import-sort/imports errors - per-file lint runs don't always agree with a whole-project pass on import ordering across an entire changed import block. No behavior change. * Proposal G, PR4b: real-browser Playwright smoke coverage 3 specs verified live in an actual browser (not jsdom - real WebCrypto, real Next.js routing, real Bootstrap modals): the editor's Save action/ breadcrumb render once signed in, the My Decks nav entry is hidden anonymously and appears once signed in, and the empty-state message renders correctly. Ran locally with a temporary executablePath override for this sandbox's browser-binary version mismatch (never run playwright install per environment policy); both playwright.config.ts and tests/global-setup.ts were reverted back to their committed state before this commit - only the new spec file is included. --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Step 1 of Proposal H's migration/sequencing plan (
docs/proposals/proposal-h-unified-display-page.md§6): a new/displayroute, entirely behindNEXT_PUBLIC_UNIFIED_DISPLAY_ENABLED(off by default — nothing changes for real users until the owner flips it). This PR also carries the Proposal H design doc + mockups themselves (previously the standalone HOLD, PR #84) — per instruction, the docs ride in with this step's flag rather than merging separately. PR #84 is superseded by this one and should be closed once this merges (no new content there beyond what's included here, plus the owner's accordion amendment which is already folded in).What shipped:
frontsVisibleview setting, not a new one), a small live subset of print settings (paper size, bleed edge, guides) that actually drives the sheet'scomputeLayout(), and a working link to the classic editor's Print tab (full inline Generate PDF/Save to Drive is Step 3 — switchover — not this PR).PagePreview/computeLayoutfrom Proposal A as-is, paginated across the whole deck (not just page 1, unlikePDFGenerator's fast preview) via a newdisplayPagination.tshelper. That helper preserves(face, slot)identity per entry —CardSelectionModeToPaginator(PDF.tsx) discards that, which this page's click-to-select needs. Only the current page's slots are ever resolved to thumbnail URLs (the "render only the current sheet page" performance rule).PagePreviewgained optionalonSlotClick/selectedSlotIndexprops (unused by existing callers, zero behavior change there) andloading="lazy"/decoding="async"on its<img>.AutofillCollapseaccordion per the owner's amendment — Choose Image open by default, Attributes/Print Options/Artist/Slot Actions collapsed — each section a labeled stub naming exactly which Step 2 PR fills it in.chunkhelper fromPDF.tsxtocommon/utils.ts(PDF.tsxre-exports it for its existing callers, zero behavior change for them) — importing anything fromPDF.tsxpulls in@react-pdf/renderer's ESM-only bundle, which Jest can't transform out of the box; this broke the new pagination helper's own unit test until moved.Deviations from spec, with reasoning
md, the rail stacks in plain document flow below the sheet (usable, not yet the polished drawer/overlay the design doc's §3 specifies). Scoped out of Step 1 ("page shell... rail skeleton") — follow-up work, not silently dropped; noted inDisplayPage.tsx's own module comment.PDFGenerator's export-time front-then-distinct-back interleaving — simpler and consistent with the editor's own existing toggle; a fully interleaved dual-face sheet is deferred (design doc §5).keyprop) was originally placed on a<div>insideRailrather than on the<Rail>element itself in its parent's JSX — since akeyonly affects remounting from the parent's perspective,expandedSectionsstate was silently persisting across slot selections instead of resetting to the documented default. Fixed; a duplicate-rail-render bug (one instance per Bootstrap breakpoint utility class, doubling every testid) was also found and fixed the same way — seeDisplayPage.tsx's inline comments for both.Test plan
chunkmove andPagePreviewprop additions are backward-compatible).displayPagination.test.ts(pagination + slot-identity preservation),featureFlags.test.ts(flag parsing),PagePreview.test.tsx's new click/selection/lazy-load cases.tests/DisplayPage.spec.ts(7 tests, all passing) exercising the real flagged route end to end: empty-state + link back to editor, live sheet rendering + pagination indicator, slot-select swapping the rail from idle, accordion defaults (Choose Image open/rest collapsed), expand-on-click, reset-on-reselecting-a-different-slot, Fronts/Backs toggle, Guides toggle showing/hiding cut-lines.PDFGenerator.spec.ts,ProjectEditorMobileScroll.spec.ts,ExportXML.spec.ts(12 tests) all still pass.tsc --noEmitclean;eslintclean (0 errors — 2 pre-existing warnings only, both predating this PR).pre-commititself isn't installed in this session's container; the checks it would run (prettier, eslint) were run manually instead (npx prettier --check/--write,npx eslint).Merge-time checklist
/displayis flag-gated off by default, so nothing user-facing has changed. Revisit once the flag flips on for real users (Step 3, switchover).Generated by Claude Code