Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-stylecache-scoped-reset.md
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 11 additions & 9 deletions packages/utilities/src/css/StyleCache.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
type Styles = Record<string, string>;

class StyleCache {
cache: Map<HTMLElement, Record<string, string>> = new Map();
cache: Map<HTMLElement, Styles> = 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);
};
}

Expand Down
34 changes: 27 additions & 7 deletions packages/utilities/src/css/tests/StyleCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,43 +19,63 @@ 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";
el2.style.color = "blue";

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);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/utilities/src/scroll/lockSafari.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const lockSafariScroll = () => {
});

return () => {
styleCache.reset();
styleCache.reset(document.body);
window.scrollTo({ top: scrollY, left: scrollX, behavior: "instant" });
};
};
Expand Down
12 changes: 6 additions & 6 deletions packages/utilities/src/scroll/lockStandard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = { 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;
31 changes: 31 additions & 0 deletions packages/utilities/src/scroll/tests/lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Loading