fix(web): full ARIA-modal isolation for the mobile detail pane (TASK-2131)#1007
Merged
Conversation
…-2131) Follow-up to TASK-2122. The mobile full-screen detail-pane overlay had a JS focus trap + an inert list column, but the app-shell chrome behind it stayed in the a11y tree and the pane was still just an <aside>. Complete the modal: - The mobile `.item-pane` becomes role="dialog" aria-modal="true"; the desktop split stays a bare <aside> (complementary landmark, non-modal). - MobileContextBar + BottomNav (rendered in the workspace +layout, ABOVE the pane host) are marked `inert` while a mobile overlay is up, so they leave the focus order AND the screen-reader tree. A JS trap can't constrain an SR virtual cursor and aria-modal is unevenly honored, so the background chrome must physically drop out. The chrome is a layout sibling the host can't reach by prop, so PaneHost hoists "a mobile overlay is active" into a small ref-counted store (paneOverlay.svelte.ts) the layout reads — one-way writer/reader split per CONVE-1688. Ref-counted so an overlapping route-change remount can't clear the signal early. The layout carries `inert` on display:contents wrappers (cascades to the fixed chrome, adds no box). Verified in a real browser (Playwright): mobile → dialog role + aria-modal + inert descendants unfocusable; desktop → bare aside + chrome interactive. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra
…ASK-2131)
The new mobile role="dialog" on the pane collided with three places that
treat ANY [role="dialog"] as a *foreign* modal that owns its own ESC /
focus — regressions the pane's <aside>-not-a-dialog invariant had been
silently relying on:
- paneFocus.ts PANE_EXEMPT_SURFACE_SELECTOR: any in-pane element matched
closest('[role="dialog"]') → the whole pane read as an "exempt surface",
killing the mobile Tab trap and confusing the focus-follows classifier.
- The collection + item-page ESC guards querySelector('[role="dialog"]')
→ the pane matched itself → ESC was swallowed instead of closing/popping
the pane on mobile.
Fix: exclude the pane via [role="dialog"]:not(.item-pane) at all three
sites (a genuinely nested dialog/menu opened FROM the pane still matches).
Adds inExemptSurface unit coverage for the pane-not-exempt case.
Caught by the independent Codex review pass.
Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra
Two more members of the same [role="dialog"] collision class, from the independent Codex pass: - Print: app.css @media print hides [role="dialog"] to strip overlays. The mobile pane now matches, so printing at <=768px with the pane open dropped the whole item from the printout. Exclude via :not(.item-pane) — the pane is the content being printed, not a transient overlay. - Banners: VerifyEmailBanner + ConnectBanner rendered OUTSIDE the inert wrappers, so their controls (Resend / Connect) stayed reachable by an SR virtual cursor behind the aria-modal pane — the same gap the inert of MobileContextBar/BottomNav closes. Fold them into the top inert wrapper so ALL app-shell siblings behind the overlay leave the a11y tree; only the pane (in children()) stays interactive. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra
xarmian
added a commit
that referenced
this pull request
Jul 22, 2026
…UG-2284) (#1009) PR #1007 (TASK-2131) added a PaneHost `$effect` that calls `paneOverlay.enter()`/`leave()` to inert the app-shell chrome behind the mobile detail-pane overlay. `enter()`'s `overlayCount += 1` READS `overlayCount` inside that tracked effect scope, so the effect took a reactive dependency on the very signal it writes: enter() dirtied the effect → it re-ran → enter()d again → `effect_update_depth_exceeded`. Svelte aborts the flush, stranding the rest of the subtree's reactivity — `paneMintForRoute` stopped recomputing, so the mobile pane (and its Back chevron) rendered EMPTY. The E2E `pane-controller` mobile-overlay tests caught it; #1007's own manual check verified the ARIA attributes but not that item content still rendered. Fix: `untrack` the count read in enter()/leave() so a write from an effect never establishes a self-dependency (the write still notifies the layout reader). The ref-count mutators are written from effects by design, so the untrack belongs in the store. Also fixes the second collision from the same #1007 change: the pane is now `role="dialog"` on mobile, so pane-controller.spec.ts:771's `[role="dialog"]` + text locator matched BOTH the pane and the BottomSheet (strict-mode violation). Target the sheet by accessible name ("Quick actions") instead — the pane's is "Item detail". The effect_update_depth_exceeded runaway only manifests under the real browser scheduler (not jsdom/vitest), so the E2E overlay tests own the loop regression; the unit tests lock the ref-count semantics. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra
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
1 participant
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.
Follow-up to TASK-2122 under PLAN-2105. Completes the mobile detail-pane overlay as a true modal.
What
.item-paneoverlay becomesrole="dialog"+aria-modal="true"; the desktop split stays a bare<aside>(complementary landmark, non-modal). Reactive toviewport.isMobile.VerifyEmailBanner,ConnectBanner,MobileContextBar,BottomNav— are markedinertwhile a mobile overlay is up, so they leave the focus order and the screen-reader tree. A JS focus trap can't constrain an SR virtual cursor andaria-modalis unevenly honored, so the background must physically drop out.paneOverlay.svelte.ts) the layout reads — one-way writer/reader split per CONVE-1688; ref-counted so an overlapping route-change remount can't clear it early. The layout carriesinertondisplay:contentswrappers (cascades to the fixed chrome, adds no box).Collision-class fixes (the pane is now itself a
role="dialog")Adding the role collided with three places that treat any
[role="dialog"]as a foreign modal. All now exclude the pane via[role="dialog"]:not(.item-pane):PANE_EXEMPT_SURFACE_SELECTOR(paneFocus.ts) — otherwise the mobile Tab trap + focus-follows classifier saw the whole pane as an "exempt surface".@media printoverlay-hide rule (app.css) — otherwise printing at ≤768px with the pane open dropped the item content.Tests
paneOverlay.svelte.test.ts— ref-count semantics (overlap, never-negative).paneFocus.svelte.test.ts—inExemptSurfacepane-not-exempt + nested-dialog-still-exempt.Runtime-verified (Playwright, real browser)
role=dialog+aria-modal=true; chrome/banner wrappersinert(injected descendant unfocusable); ESC reaches close (?itemdrops); Tab stays inside the pane; pane stays printable in print media.<aside>(no role/aria-modal), chrome interactive.Review
Three independent Codex passes (orchestrator-run). Round 1 found 2 P1s (ESC swallow, Tab-trap break); round 2 found the print regression + the banner gap — all fixed here. Round 3 confirmed the
[role="dialog"]class is fully swept.Deferred (follow-up)
The root-layout
Sidebar's collapsed mobile drawer is off-screen (translateX(-100%)+pointer-events:none) but still in the a11y tree — a pre-existing condition on every mobile screen, independent of this change and mitigated here byaria-modal. The proper fix belongs in the Sidebar (inert the collapsed drawer regardless of the pane); filed as a separate item.https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra