diff --git a/.changeset/fix-stylecache-scoped-reset.md b/.changeset/fix-stylecache-scoped-reset.md new file mode 100644 index 00000000..e2c88450 --- /dev/null +++ b/.changeset/fix-stylecache-scoped-reset.md @@ -0,0 +1,5 @@ +--- +"@reshaped/utilities": patch +--- + +StyleCache: `reset()` used to restore and clear the entire cache, which let one scroll lock wipe the saved styles of another still-active lock. It now takes a required element and restores only that one. `set()` is also a no-op when the element is already cached, so locking an already-locked element keeps its original styles intact. diff --git a/packages/utilities/src/css/StyleCache.ts b/packages/utilities/src/css/StyleCache.ts index 8a694fbb..036424e3 100644 --- a/packages/utilities/src/css/StyleCache.ts +++ b/packages/utilities/src/css/StyleCache.ts @@ -1,26 +1,28 @@ type Styles = Record; class StyleCache { - cache: Map> = new Map(); + cache: Map = new Map(); set = (el: HTMLElement, styles: Styles) => { - const originalStyles: Styles = {}; - const cachedStyles = this.cache.get(el); + // Already cached (locked) — keep the originals captured the first time and + // don't reapply, so locking an element that's already locked is a no-op. + if (this.cache.has(el)) return; + const originalStyles: Styles = {}; Object.keys(styles).forEach((key) => { originalStyles[key] = el.style.getPropertyValue(key); }); - this.cache.set(el, { ...originalStyles, ...cachedStyles }); + this.cache.set(el, originalStyles); Object.assign(el.style, styles); }; - reset = () => { - for (const [el, styles] of this.cache.entries()) { - Object.assign(el.style, styles); - } + reset = (el: HTMLElement) => { + const styles = this.cache.get(el); + if (!styles) return; - this.cache.clear(); + Object.assign(el.style, styles); + this.cache.delete(el); }; } diff --git a/packages/utilities/src/css/tests/StyleCache.test.ts b/packages/utilities/src/css/tests/StyleCache.test.ts index a4a17642..c6b311f6 100644 --- a/packages/utilities/src/css/tests/StyleCache.test.ts +++ b/packages/utilities/src/css/tests/StyleCache.test.ts @@ -19,24 +19,28 @@ describe("css/StyleCache", () => { expect(el.style.color).toBe("blue"); expect(el.style.overflow).toBe("hidden"); - styleCache.reset(); + styleCache.reset(el); expect(el.style.color).toBe("red"); expect(el.style.overflow).toBe("visible"); }); - test("preserves original styles across multiple set calls", () => { + test("ignores repeated set calls on an already-cached element", () => { const el = document.createElement("div"); el.style.color = "red"; styleCache.set(el, { color: "blue" }); + // The element is already cached, so this is a no-op (style stays "blue") styleCache.set(el, { color: "green" }); - styleCache.reset(); + + expect(el.style.color).toBe("blue"); + + styleCache.reset(el); expect(el.style.color).toBe("red"); }); - test("handles multiple elements", () => { + test("reset only affects the given element", () => { const el1 = document.createElement("div"); const el2 = document.createElement("div"); el1.style.color = "red"; @@ -44,18 +48,34 @@ describe("css/StyleCache", () => { styleCache.set(el1, { color: "green" }); styleCache.set(el2, { color: "yellow" }); - styleCache.reset(); + styleCache.reset(el1); + + // el1 is restored, el2 stays locked expect(el1.style.color).toBe("red"); + expect(el2.style.color).toBe("yellow"); + expect(styleCache.cache.has(el2)).toBe(true); + + styleCache.reset(el2); + expect(el2.style.color).toBe("blue"); }); - test("clears cache after reset", () => { + test("reset is a no-op for an element that was never cached", () => { + const el = document.createElement("div"); + el.style.color = "red"; + + styleCache.reset(el); + + expect(el.style.color).toBe("red"); + }); + + test("removes the element from the cache after reset", () => { const el = document.createElement("div"); el.style.color = "red"; styleCache.set(el, { color: "blue" }); - styleCache.reset(); + styleCache.reset(el); expect(styleCache.cache.size).toBe(0); }); diff --git a/packages/utilities/src/scroll/lockSafari.ts b/packages/utilities/src/scroll/lockSafari.ts index 9ae40362..36c58f2e 100644 --- a/packages/utilities/src/scroll/lockSafari.ts +++ b/packages/utilities/src/scroll/lockSafari.ts @@ -17,7 +17,7 @@ const lockSafariScroll = () => { }); return () => { - styleCache.reset(); + styleCache.reset(document.body); window.scrollTo({ top: scrollY, left: scrollX, behavior: "instant" }); }; }; diff --git a/packages/utilities/src/scroll/lockStandard.ts b/packages/utilities/src/scroll/lockStandard.ts index 906ed960..7f73802d 100644 --- a/packages/utilities/src/scroll/lockStandard.ts +++ b/packages/utilities/src/scroll/lockStandard.ts @@ -6,19 +6,19 @@ const styleCache = new StyleCache(); const lockStandardScroll = (args: { container: HTMLElement }) => { const { container } = args; const isOverflowing = container.scrollHeight > container.clientHeight; - - styleCache.set(container, { overflow: "hidden" }); + const styles: Record = { overflow: "hidden" }; if (isOverflowing) { if (CSS.supports("scrollbar-gutter", "stable")) { - styleCache.set(container, { scrollbarGutter: "stable" }); + styles.scrollbarGutter = "stable"; } else { - const scrollBarWidth = getScrollbarWidth(); - styleCache.set(container, { paddingRight: `${scrollBarWidth}px` }); + styles.paddingRight = `${getScrollbarWidth()}px`; } } - return () => styleCache.reset(); + styleCache.set(container, styles); + + return () => styleCache.reset(container); }; export default lockStandardScroll; diff --git a/packages/utilities/src/scroll/tests/lock.test.ts b/packages/utilities/src/scroll/tests/lock.test.ts index 7e8e0f59..213583f0 100644 --- a/packages/utilities/src/scroll/tests/lock.test.ts +++ b/packages/utilities/src/scroll/tests/lock.test.ts @@ -137,6 +137,37 @@ describe("scroll/lockScroll", () => { expect(document.documentElement.style.overflow).toBe(""); }); + test("unlocking one container does not affect another locked container", () => { + const a = document.createElement("div"); + a.style.overflow = "auto"; + a.style.height = "100px"; + document.body.appendChild(a); + + const b = document.createElement("div"); + b.style.overflow = "scroll"; + b.style.height = "100px"; + document.body.appendChild(b); + + const unlockA = lockScroll({ containerEl: a }); + const unlockB = lockScroll({ containerEl: b }); + + expect(a.style.overflow).toBe("hidden"); + expect(b.style.overflow).toBe("hidden"); + + unlockA?.(); + + // B shares the module-level StyleCache with A, but must stay locked + expect(a.style.overflow).toBe("auto"); + expect(b.style.overflow).toBe("hidden"); + + unlockB?.(); + + expect(b.style.overflow).toBe("scroll"); + + document.body.removeChild(a); + document.body.removeChild(b); + }); + test("calls lock callback immediately", () => { const lockCb = vi.fn();