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..04c42fec 100644 --- a/packages/utilities/src/scroll/lock.ts +++ b/packages/utilities/src/scroll/lock.ts @@ -3,13 +3,14 @@ import { isIOS } from "@/platform"; import lockSafariScroll from "./lockSafari"; import lockStandardScroll from "./lockStandard"; +const locks = 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 +18,27 @@ 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; - - if (isIOSLock && lockedDocumentScroll) { - reset = lockSafariScroll(); - } else { - reset = lockStandardScroll({ container }); + let lock = locks.get(container); + if (!lock) { + const reset = + isIOSLock && lockedDocumentScroll ? lockSafariScroll() : lockStandardScroll({ container }); + lock = { count: 0, reset }; + locks.set(container, lock); } + lock.count++; args?.callback?.(); - return (args?: { callback?: () => void }) => { - reset(); - args?.callback?.(); + let released = false; + return (unlockArgs?: { callback?: () => void }) => { + if (released) return; + released = true; + + 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", () => {