Skip to content

fix(scroll): reference-count locks so stacked locks release correctly#640

Merged
blvdmitry merged 2 commits into
canaryfrom
claude/fix-lockscroll-refcount
Jun 22, 2026
Merged

fix(scroll): reference-count locks so stacked locks release correctly#640
blvdmitry merged 2 commits into
canaryfrom
claude/fix-lockscroll-refcount

Conversation

@blvdmitry

Copy link
Copy Markdown
Contributor

Problem

lockScroll short-circuited when the container was already locked:

// Already locked so no need to lock again and trigger the callback
if (container.style.overflow === "hidden") return;

This returns undefined, so a second locker (e.g. a modal opening a dropdown, or two modals both locking the document) gets no unlock function and its callback never fires. And because there's no tracking of who holds the lock, releasing the first lock restores the scroll while the second is still open — the page becomes scrollable under an open overlay, and the second overlay has no way to unlock.

Fix

Reference-count active locks per container (via WeakMap): apply the style changes only for the first lock, restore only when the last lock is released, and always return a valid, idempotent unlock function.

const lockCounts = new WeakMap<HTMLElement, number>();
const lockResets = new WeakMap<HTMLElement, () => void>();
// ...
if (currentCount === 0) { /* apply lock, store reset */ }
lockCounts.set(container, currentCount + 1);
// unlock(): decrement; when it hits 0, run the stored reset

Behavioural note: lockScroll now always returns an unlock function (previously it could return undefined). Callers that did unlock?.() are unaffected.

Verification

Logic simulated:

after 2 locks: applied=1 reset=0   (styles applied once)
after unlock A: reset=0            (B still open -> still locked)
after unlock B: reset=1            (last unlock -> restored)
double unlock A: reset=1           (idempotent)
  • tsc --noEmit (utilities) → 0 errors. oxlint → clean.

How to test

  1. Open a Modal, then from within it open a Dropdown/another Modal (both lock the document).
  2. Close them in either order.
  3. Before: closing the first re-enables page scroll while the second is still open; the second may never restore scroll. After: scroll stays locked until the last overlay closes, then restores.

Found during a full package bug review. Complementary to the StyleCache-scoping and iOS-guard PRs in the same cluster.


Generated by Claude Code

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Library / JS 48.71 KB (0%) 975 ms (0%) 312 ms (+7.44% 🔺) 1.3 s
Library / CSS 22.18 KB (0%) 444 ms (0%) 0 ms (+100% 🔺) 444 ms
Theming / JS 7.81 KB (0%) 157 ms (0%) 257 ms (+442.13% 🔺) 413 ms
Theming with a default theme definition / JS 8.65 KB (0%) 174 ms (0%) 49 ms (+2.99% 🔺) 222 ms

lockScroll early-returned undefined when the container was already locked,
so a second locker (e.g. a modal opening a dropdown, or two modals on the
document) got no unlock function and its callback never fired. Worse,
releasing the first lock restored the scroll while the second was still
open. Track an active-lock count per container: apply styles only for the
first lock, restore only when the last lock is released, and always return
a valid (idempotent) unlock function.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01We9NqptZjfbvXRL4hE1xri
@blvdmitry blvdmitry force-pushed the claude/fix-lockscroll-refcount branch from 62819da to e46a561 Compare June 20, 2026 09:23
@blvdmitry blvdmitry marked this pull request as ready for review June 22, 2026 20:20
@blvdmitry blvdmitry merged commit 4aaa491 into canary Jun 22, 2026
2 checks passed
@blvdmitry blvdmitry deleted the claude/fix-lockscroll-refcount branch June 22, 2026 21:39
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.

2 participants