Skip to content

fix(web): stop paneOverlay ref-count effect self-looping on mobile (BUG-2284)#1009

Merged
xarmian merged 1 commit into
mainfrom
fix/paneoverlay-effect-loop
Jul 22, 2026
Merged

fix(web): stop paneOverlay ref-count effect self-looping on mobile (BUG-2284)#1009
xarmian merged 1 commit into
mainfrom
fix/paneoverlay-effect-loop

Conversation

@xarmian

@xarmian xarmian commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

What

Fixes the red E2E on main — two failures introduced by #1007 (TASK-2131, "full ARIA-modal isolation for the mobile detail pane"), both stemming from making the mobile pane role="dialog".

1. The real defect — mobile pane renders empty (effect_update_depth_exceeded)

#1007 added a PaneHost $effect that calls paneOverlay.enter()/leave() to inert the app-shell chrome behind the mobile 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:

effect runs → enter() writes overlayCount → effect dirty → re-run → enter() → … → effect_update_depth_exceeded

Svelte aborts the flush, which strands the rest of the subtree's reactivity: paneMintForRoute (in the collection page) stopped recomputing, so {#if paneMintForRoute} stayed false and the mobile pane rendered completely empty — no item content, no Back chevron. #1007's manual Playwright 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's mobileOverlayActive reader normally. The ref-count mutators are written from effects by design, so the untrack belongs in the store (matches the existing untrack-the-write idiom in UserSettingsForm.svelte / Lightbox.svelte).

2. Test-locator collision

The pane is now role="dialog" on mobile, so pane-controller.spec.ts:771's [role="dialog"] + text locator matched both the pane (which contains the sheet) and the BottomSheet → strict-mode violation. Target the sheet by accessible name (getByRole('dialog', { name: 'Quick actions' })) — the pane's is "Item detail".

Diagnosis

Bisected the red E2E to #1007 (parent 316e25a6 green, b76abdd6 red). Instrumented the collection page + PaneHost to trace the nav/mint sequence and captured the effect_update_depth_exceeded pageerror; confirmed by isolation that disabling the overlay effect removed the loop, and that restoring it with untrack keeps #1007's a11y isolation working.

Verification

  • cd web && npm run check — svelte-check 0 errors
  • paneOverlay.svelte.test.ts4 pass (ref-count semantics)
  • pane-controller (771 + 1178) now pass; pane-controller + pane-a11y-focus + pane-mobile-overlay suite — 28 pass (1 unrelated documented tie-break flake, BUG-2270/2279)
  • Codex CLI review — CLEAN

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.

Fixes BUG-2284. Unblocks #1008 (IDEA-2135), whose E2E inherited this red.

https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra

…UG-2284)

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
@xarmian
xarmian merged commit 8c710e1 into main Jul 22, 2026
6 checks passed
@xarmian
xarmian deleted the fix/paneoverlay-effect-loop branch July 22, 2026 23:30
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