Skip to content

Port ChangeQueryModal/Toasts/a11y/InvalidIdentifiers/SelectVersionSection clusters onto DisplayPage - #403

Merged
WilfordGrimley merged 1 commit into
proposal-h-editor-display-route-swapfrom
parity-wave2-changequery-toasts-a11y
Jul 24, 2026
Merged

Port ChangeQueryModal/Toasts/a11y/InvalidIdentifiers/SelectVersionSection clusters onto DisplayPage#403
WilfordGrimley merged 1 commit into
proposal-h-editor-display-route-swapfrom
parity-wave2-changequery-toasts-a11y

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

Wave 2 of issue #272's parity work (following wave 1's PR #395): un-skips and
ports 44 Playwright tests from the classic /editor grid's coverage onto the
unified DisplayPage at /editor (PR #389's own swap), stacked on
proposal-h-editor-display-route-swap (PR #389) the same way wave 1 was.

Ported clusters (44 tests)

  1. SelectVersionSection.spec.ts (7 tests) - this file was never actually
    classic-editor-only: it already exercised the unified page's own rail (via
    openSelectVersionSection, added to test-utils.ts back when
    DisplayLeftRailFidelity.spec.ts landed) since issue Select Version section build (blocked on PR #116) #167 shipped, well
    before the route swap. It picked up the swap's blanket per-file skip
    marker anyway and was deliberately left for this wave. Un-skipped with no
    other change - all 7 pass as-is.
  2. ChangeQueryModal.spec.ts (9 of 12 tests) - ChangeQueryModal.tsx is
    unchanged and still globally mounted (Modals.tsx); reached via the
    shared CardSlotContextMenu's "Change Query" action (right-click a sheet
    slot), the same context menu DisplayPage.tsx already wires up
    ("no new action, just three more ways to reach it on /display" per that
    file's own comment). New openDisplayChangeQueryModal helper in
    test-utils.ts.
  3. Toasts.spec.ts (3 of 9 tests, the rest were already passing) - the
    DFCPairs/importSites/sampleCards error-toast tests used
    openImportTextModal/getAddCardsMenu only as a means to mount
    ImportText.tsx/ImportURL.tsx, whose own RTK Query hooks fire on
    mount, not on any click. DisplayPage's empty landing mounts both
    unconditionally (ImportColumns, URL accordion defaultActiveKey="url")
    • all three fetches already fire on plain page load, same as the
      already-passing /2/cards//2/sources tests beside them.
  4. GeneralUIAccessibility.spec.ts (3 of 4 tests) - the per-slot "more
    options" a11y checks retarget onto PagePreview.tsx's own per-slot menu
    cue (page-preview-slot-menu-cue, "Open card menu" - the sheet's closest
    equivalent, reserving the same corner classic CardSlot's 3-dot button
    occupied). The editor-page console-warnings test ports directly.
  5. InvalidIdentifiersStatus.spec.ts (5 tests) + its visual companion (1
    test)
    - InvalidIdentifiersStatus.tsx/InvalidIdentifiersModal.tsx are
    both unchanged; DisplayPage.tsx's own D13 comment confirms the
    landing/search-bar mount (which these tests exercise) already works -
    only the separate right-rail Status row (issue /display parity inventory: verified remaining gaps vs /editor (switchover checklist) #272 item 2's own
    remaining scope) is missing.
  6. SearchSettings.visual.spec.ts (1 test) - SearchSettings.tsx is
    unchanged, relocated into the right rail unmodified. New
    openDisplaySearchSettingsModal helper (opens the gear button/right
    rail first, when it isn't already inline).

Deviations from a clean port

  • 3 of ChangeQueryModal.spec.ts's 12 tests dropped: the multi-slot
    DFC tests ("plural text when multiple front slots are selected",
    "updates all fronts and backs for a multi-slot selection", "not shown
    when any one of the selected slots already has the DFC back query")
    depended on the classic grid's checkbox multi-select +
    SelectedImagesRibbon's own "Change Query" trigger - bulk multi-select
    has no equivalent on the unified page (see "Parked clusters" below). The
    9 single-slot DFC prompt/submission/condition/fuzzy-search/checkbox-reset
    tests are unaffected and fully prove the same underlying logic.
  • GeneralUIAccessibility.spec.ts's "comfortable touch-target size"
    (>=40x40px) assertion dropped
    , not weakened: PagePreview.tsx renders
    the sheet near print-scale (millimetre-driven sizing), and the menu cue
    measured ~11x11px there - a real, structural difference in what the
    element IS (a small on-page-preview affordance vs. a full-size toolbar
    button), not an accessibility regression. The focus-visible-style half of
    that same test is unaffected and still verified.
  • GeneralUIAccessibility.spec.ts's two "Editor - mobile layout" tests
    dropped
    (left-panel/right-panel 50/50-split-vs-stacked): they verified
    the same "mobile scroll affordances -> Mobile /display: fit-to-width sheet + bottom-sheet drawer for card actions (#231 blocker) #266's responsive layer"
    replacement issue /display parity inventory: verified remaining gaps vs /editor (switchover checklist) #272's own body already lists as intentional, not a
    gap. The unified page's rails are off-canvas drawers below their own
    breakpoints regardless of viewport - there's no persistent 50/50 split to
    ever "stack" in the first place.
  • CardImageStates.spec.ts investigated, left skipped with an improved
    reason
    - not a route-swap casualty after all: its
    card-image-error-placeholder/card-image-slow-load-hint testids only
    exist in Card.tsx. PagePreview.tsx's sheet slots are their own
    component with their own loading/failed states (already covered by
    DisplaySlotStates.spec.ts) but a plain, unwrapped <img> for the
    "candidate resolved, but its own image fetch 404s or hangs" case - no
    error-placeholder swap, no slow-load hint, at all. This is a genuine,
    confirmed gap (new UI work, not test-porting), not something this wave
    can port.
  • DeckbuilderConfirmAffordance.spec.ts investigated, left skipped -
    its rail-header mount was intentionally REMOVED and superseded by
    ConfidenceElement.tsx (DisplayPage.tsx's own D14 comment). Its only
    remaining reachable mount (the Select Version confirm ribbon) is already
    covered by SelectVersionSection.spec.ts's own "carries the confirm
    ribbon" test, ported above - porting this file too would just duplicate
    that coverage against a UI surface (the auto-shown top-level badge) that
    no longer exists on this page.

Parked clusters (skip left untouched, not ported or dropped)

I could not find an authoritative "cluster N" list in issue #272 itself
(its own checklist is 8 numbered GAPS, not test clusters) - the mapping
below is my own inference from the issue body's "Intentional replacements,
NOT gaps" list plus source investigation. Flagging this so it can be
corrected if it doesn't match the intended list:

Deferred to a future wave (not owner-parked - time/complexity only)

  • GridSelectorModal.spec.ts (23 tests) + GridSelectorModalVariants.spec.ts
    (7 tests)
    - the modal is reachable on DisplayPage (e.g. via
    CardbackToolbarButton), but mapping this cluster's grouping/filter/
    title-count assertions onto a genuinely reachable instance is real,
    non-mechanical work I didn't get to this wave.
  • CardSlot.spec.ts (25 tests) + visual/CardSlot.visual.spec.ts -
    large per-slot interaction cluster (image cycling arrows, per-slot
    context menu, badges) that needs redesigning around the sheet+rail
    architecture rather than a 1:1 retarget.

Saying this plainly rather than silently shrinking wave 2's scope: these
two clusters are real, non-trivial remaining work, not confirmed gaps or
owner decisions.

Verification

  • All 44 newly-unskipped tests green, run individually per-cluster and
    together.
  • Wave 1's marquee regression files re-run: ImportXML.spec.ts (16),
    chunkErrorRecovery.spec.ts, DisplayLeftRailFidelity.spec.ts - all
    pass.
  • Full suite (npx playwright test, excl. perf): 325 total, 225 passed, 96
    skipped (unchanged - only intra-file skips converted to real tests, no
    file went from unskipped to skipped), 4 failed on the first full run
    (Navbar.spec.ts's Sign-out test, UnsavedWorkGuard.spec.ts's
    editor->/print test, visual/CardDetailedViewModal.visual.spec.ts,
    visual/DynamicLogo.visual.spec.ts) - all 4 re-run individually and
    passed; none touch a file this PR changed. DynamicLogo.visual.spec.ts's
    failure in the full run is this session's own dedicated-port workaround
    (its MSW handler hardcodes localhost:3000), documented in wave 1's own
    PR as the same known artifact.
  • npx tsc --noEmit clean.
  • npx jest --silent - 568/568 passing (unchanged; no src/ files
    touched).
  • npx prettier@2.7.1 --check clean on all changed files.

Environment note: local runs used a temporary, uncommitted
playwright.config.ts port override (3411) per docs/lessons.md's
worktree-port-collision entry, reverted before every commit.

Open items for the owner

  1. The "cluster N" mapping in the Parked section above is inferred, not
    sourced from an explicit list - please correct if it's wrong.
  2. GridSelectorModal.spec.ts/GridSelectorModalVariants.spec.ts/
    CardSlot.spec.ts (55 tests total) remain deferred, not parked - real
    scope for a wave 3.
  3. CardImageStates.spec.ts's gap (no image-fetch-error/slow-load UX on
    the sheet) may be worth its own tracked issue, separate from /display parity inventory: verified remaining gaps vs /editor (switchover checklist) #272.

Checklist

  • I have installed pre-commit and installed the hooks with
    pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new
    tests where appropriate. (This PR is the test update.)
  • I have manually tested my changes as follows: see Verification
    above.
  • I have updated any relevant documentation or created new
    documentation where appropriate. (No docs/ file tracks per-wave
    test-porting status, matching wave 1's own precedent.)

Stacked on proposal-h-editor-display-route-swap (PR #389, held open,
parity-blocked), same stacking as wave 1's PR #395.

…tion clusters onto DisplayPage

Wave 2 of issue #272: un-skips and ports 44 Playwright tests from the classic
/editor grid onto the unified DisplayPage (issue #272 wave 2): SelectVersionSection
(7, already-adapted, just needed its skip removed), ChangeQueryModal via the shared
CardSlotContextMenu (9), 3 Toasts.spec.ts tests that fire on plain landing-page mount,
GeneralUIAccessibility's per-slot a11y checks (3), InvalidIdentifiersStatus (5) +
its visual companion, and SearchSettings.visual (1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@WilfordGrimley
WilfordGrimley merged commit e61ca68 into proposal-h-editor-display-route-swap Jul 24, 2026
7 checks passed
WilfordGrimley added a commit that referenced this pull request Jul 24, 2026
* Swap Proposal H's unified page onto /editor, redirect /display

Nav-redesign PR #313 only pointed the "Editor" nav label at /display
while the classic grid page stayed live at /editor - per the owner's
explicit directive, this completes the actual route swap: the unified
sheet+rail page now serves at /editor, /display is a client-side
redirect preserving query params/hash, and the classic ProjectEditor
is fully unrouted (component kept in-tree, deletion is a later
decision).

Ports ProjectEditor's beforeunload unsaved-work guard onto DisplayPage
(it lived only in the now-unrouted component, so it wasn't naturally
inherited). Fixes a real, unrelated redirect-preservation bug found in
useBackendSetter.ts while verifying (documented in troubleshooting.md,
not fixed here - pre-existing, affects every page, out of scope).

~190 Playwright tests that exercised classic-/editor-only UI (CardSlot
grid, SelectedImagesRibbon bulk-select, GridSelectorModal-as-modal,
etc. - issue #272's own tracked parity gaps) are now individually
skipped with a standard citation rather than left red or silently
deleted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Port chunkErrorRecovery/DisplayLeftRailFidelity specs to the unified-page route (proposal-h)

* Port import/card-detail/export test clusters onto DisplayPage (#395)

* Port parity wave 1: import/card-detail/export test clusters onto DisplayPage

Un-skips and ports 58 tests from the classic /editor grid onto the unified
page (issue #272 wave 1): ImportCSV/ImportText/ImportXML + visual (29),
the CardDetailedViewModal ecosystem via Browse mode (20), ExportDecklist/
ExportXML content-correctness (6), AddCardToProjectForm (3).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fix orphan-cardback test's dangling importXML reference in ImportXML.spec.ts

* Adapt orphan-cardback test's badge assertion to the sheet's per-slot corner badge (#398)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* Port ChangeQueryModal/Toasts/a11y/InvalidIdentifiers/SelectVersionSection clusters onto DisplayPage (#403)

Wave 2 of issue #272: un-skips and ports 44 Playwright tests from the classic
/editor grid onto the unified DisplayPage (issue #272 wave 2): SelectVersionSection
(7, already-adapted, just needed its skip removed), ChangeQueryModal via the shared
CardSlotContextMenu (9), 3 Toasts.spec.ts tests that fire on plain landing-page mount,
GeneralUIAccessibility's per-slot a11y checks (3), InvalidIdentifiersStatus (5) +
its visual companion, and SearchSettings.visual (1).

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* Port pin-persistence tests to unified-page routing

* Port GridSelectorModal/CardSlot clusters onto DisplayPage (#410)

* Port GridSelectorModal/CardSlot clusters onto DisplayPage (issue #272 wave 3)

Un-skips 30 of 55 deferred tests (GridSelectorModal.spec.ts +
GridSelectorModalVariants.spec.ts fully, CardSlot.spec.ts partially),
retargeted onto CardbackToolbarButton's cardback picker and the sheet's
page-preview-slot context menu; ports 2 of 6 CardSlot.visual.spec.ts
aria-snapshot tests, regex-tolerant on unrelated pre-existing icon-font
rendering leaves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Fold the DOM-API sheet gap into the doc body per edit-in-place convention

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant