From e46a561e8e219d0509fca0b71a4ab0bfa4af2702 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 20 Jun 2026 09:08:03 +0000 Subject: [PATCH 1/2] fix(scroll): reference-count locks so stacked locks release correctly 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 Claude-Session: https://claude.ai/code/session_01We9NqptZjfbvXRL4hE1xri --- .changeset/fix-lockscroll-refcount.md | 5 ++++ packages/utilities/src/scroll/lock.ts | 36 +++++++++++++++++++-------- 2 files changed, 31 insertions(+), 10 deletions(-) create mode 100644 .changeset/fix-lockscroll-refcount.md diff --git a/.changeset/fix-lockscroll-refcount.md b/.changeset/fix-lockscroll-refcount.md new file mode 100644 index 00000000..3418f67c --- /dev/null +++ b/.changeset/fix-lockscroll-refcount.md @@ -0,0 +1,5 @@ +--- +"@reshaped/utilities": patch +--- + +lockScroll: Fixed stacked/nested scroll locks releasing the scroll prematurely by reference-counting locks per container diff --git a/packages/utilities/src/scroll/lock.ts b/packages/utilities/src/scroll/lock.ts index 6ccdc3e1..7f8970f5 100644 --- a/packages/utilities/src/scroll/lock.ts +++ b/packages/utilities/src/scroll/lock.ts @@ -3,13 +3,15 @@ import { isIOS } from "@/platform"; import lockSafariScroll from "./lockSafari"; import lockStandardScroll from "./lockStandard"; +const lockCounts = new WeakMap(); +const lockResets = new WeakMap void>(); + export const lockScroll = (args?: { containerEl?: HTMLElement | null; originEl?: HTMLElement | null; callback?: () => void; }) => { const isIOSLock = isIOS(); - let reset = () => {}; const container = args?.containerEl ?? @@ -17,19 +19,33 @@ export const lockScroll = (args?: { document.documentElement; const lockedDocumentScroll = container === document.documentElement; - // Already locked so no need to lock again and trigger the callback - if (container.style.overflow === "hidden") return; + const currentCount = lockCounts.get(container) ?? 0; - if (isIOSLock && lockedDocumentScroll) { - reset = lockSafariScroll(); - } else { - reset = lockStandardScroll({ container }); + if (currentCount === 0) { + const reset = + isIOSLock && lockedDocumentScroll + ? lockSafariScroll() + : lockStandardScroll({ container }); + lockResets.set(container, reset); } + lockCounts.set(container, currentCount + 1); args?.callback?.(); - return (args?: { callback?: () => void }) => { - reset(); - args?.callback?.(); + let released = false; + return (unlockArgs?: { callback?: () => void }) => { + if (released) return; + released = true; + + const nextCount = (lockCounts.get(container) ?? 1) - 1; + if (nextCount <= 0) { + lockCounts.delete(container); + lockResets.get(container)?.(); + lockResets.delete(container); + } else { + lockCounts.set(container, nextCount); + } + + unlockArgs?.callback?.(); }; }; From a0f230a4d72e7fc854461ffebd3140cd607c4a39 Mon Sep 17 00:00:00 2001 From: Dmitry Belyaev Date: Mon, 22 Jun 2026 23:38:59 +0200 Subject: [PATCH 2/2] lock scroll --- packages/utilities/src/scroll/lock.ts | 27 +++++++------------ .../utilities/src/scroll/tests/lock.test.ts | 22 +++++++++++++-- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/packages/utilities/src/scroll/lock.ts b/packages/utilities/src/scroll/lock.ts index 7f8970f5..04c42fec 100644 --- a/packages/utilities/src/scroll/lock.ts +++ b/packages/utilities/src/scroll/lock.ts @@ -3,8 +3,7 @@ import { isIOS } from "@/platform"; import lockSafariScroll from "./lockSafari"; import lockStandardScroll from "./lockStandard"; -const lockCounts = new WeakMap(); -const lockResets = new WeakMap void>(); +const locks = new WeakMap void }>(); export const lockScroll = (args?: { containerEl?: HTMLElement | null; @@ -19,17 +18,15 @@ export const lockScroll = (args?: { document.documentElement; const lockedDocumentScroll = container === document.documentElement; - const currentCount = lockCounts.get(container) ?? 0; - - if (currentCount === 0) { + let lock = locks.get(container); + if (!lock) { const reset = - isIOSLock && lockedDocumentScroll - ? lockSafariScroll() - : lockStandardScroll({ container }); - lockResets.set(container, reset); + isIOSLock && lockedDocumentScroll ? lockSafariScroll() : lockStandardScroll({ container }); + lock = { count: 0, reset }; + locks.set(container, lock); } - lockCounts.set(container, currentCount + 1); + lock.count++; args?.callback?.(); let released = false; @@ -37,13 +34,9 @@ export const lockScroll = (args?: { if (released) return; released = true; - const nextCount = (lockCounts.get(container) ?? 1) - 1; - if (nextCount <= 0) { - lockCounts.delete(container); - lockResets.get(container)?.(); - lockResets.delete(container); - } else { - lockCounts.set(container, nextCount); + if (--lock.count <= 0) { + lock.reset(); + locks.delete(container); } unlockArgs?.callback?.(); diff --git a/packages/utilities/src/scroll/tests/lock.test.ts b/packages/utilities/src/scroll/tests/lock.test.ts index e9edef2d..056aa9cf 100644 --- a/packages/utilities/src/scroll/tests/lock.test.ts +++ b/packages/utilities/src/scroll/tests/lock.test.ts @@ -73,14 +73,30 @@ describe("scroll/lockScroll", () => { expect(scrollableContainer.style.overflow).toBe("auto"); }); - test("unlocks after multiple locks", () => { + test("keeps scroll locked until every stacked lock is released", () => { const unlock1 = lockScroll({}); const unlock2 = lockScroll({}); expect(document.documentElement.style.overflow).toBe("hidden"); + // The first release must not unlock while a second lock is still held unlock1?.(); + expect(document.documentElement.style.overflow).toBe("hidden"); + + // Only the last release actually unlocks the container + unlock2?.(); expect(document.documentElement.style.overflow).toBe(""); + }); + + test("ignores repeated calls to the same unlock", () => { + const unlock1 = lockScroll({}); + const unlock2 = lockScroll({}); + + // Calling the first unlock twice must not decrement the count for unlock2 + unlock1?.(); + unlock1?.(); + + expect(document.documentElement.style.overflow).toBe("hidden"); unlock2?.(); expect(document.documentElement.style.overflow).toBe(""); @@ -89,9 +105,11 @@ describe("scroll/lockScroll", () => { test("calls lock callback immediately", () => { const lockCb = vi.fn(); - lockScroll({ callback: lockCb }); + const unlock = lockScroll({ callback: lockCb }); expect(lockCb).toHaveBeenCalledTimes(1); + + unlock?.(); }); test("calls unlock callback when unlocking", () => {