Skip to content

Commit 9ccb894

Browse files
segunadebayosegunadebayo
committed
fix(remove-scroll): lock html when it owns viewport scroll
Apply overflow locking to documentElement when it is the scroll container, instead of always targeting body.
1 parent 75ee543 commit 9ccb894

7 files changed

Lines changed: 163 additions & 4 deletions

File tree

‎.changeset/remove-scroll-html-scroller.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@zag-js/remove-scroll": patch
3+
---
4+
5+
- Fixed issue where the scroll lock could apply to the wrong element (`<body>`) on layouts where `<html>` is the actual scroll container, silently doing nothing.

‎e2e/models/scroll-lock.model.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { expect, type Page } from "@playwright/test"
2+
import { Model } from "./model"
3+
4+
export class ScrollLockModel extends Model {
5+
constructor(public page: Page) {
6+
super(page)
7+
}
8+
9+
gotoHtmlScroller() {
10+
return this.page.goto("/scroll-lock/html-scroller")
11+
}
12+
13+
clickTrigger() {
14+
return this.page.getByTestId("dialog-trigger").click()
15+
}
16+
17+
clickClose() {
18+
return this.page.getByTestId("dialog-close").click()
19+
}
20+
21+
async seeZagLockEventually() {
22+
await expect(this.page.locator("body")).toHaveAttribute("data-scroll-lock", "")
23+
}
24+
25+
async seeNoZagLockEventually() {
26+
await expect(this.page.locator("body")).not.toHaveAttribute("data-scroll-lock", "")
27+
}
28+
29+
isHtmlLocked() {
30+
return this.page.evaluate(() => document.documentElement.style.overflow === "hidden")
31+
}
32+
33+
isBodyLocked() {
34+
return this.page.evaluate(() => document.body.style.overflow === "hidden")
35+
}
36+
}

‎e2e/scroll-lock.e2e.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { expect, test } from "@playwright/test"
2+
import { ScrollLockModel } from "./models/scroll-lock.model"
3+
4+
let I: ScrollLockModel
5+
6+
test.describe("scroll lock", () => {
7+
test.beforeEach(async ({ page }) => {
8+
I = new ScrollLockModel(page)
9+
})
10+
11+
test.describe("scroll container targeting", () => {
12+
test.beforeEach(async () => {
13+
await I.gotoHtmlScroller()
14+
})
15+
16+
test("locks <html> instead of <body> when html establishes the scroll container", async () => {
17+
await I.clickTrigger()
18+
await I.seeZagLockEventually() // the data-scroll-lock marker is always on body
19+
20+
expect(await I.isHtmlLocked()).toBe(true)
21+
expect(await I.isBodyLocked()).toBe(false)
22+
})
23+
24+
test("locks immediately when an external <body> lock cannot affect the page", async () => {
25+
// html owns the viewport scroll here; an overflow:hidden on body alone doesn't actually
26+
// lock anything and must not prevent zag from locking the real scroller.
27+
await I.page.evaluate(() => (document.body.style.overflowY = "hidden"))
28+
29+
await I.clickTrigger()
30+
await I.seeZagLockEventually()
31+
32+
expect(await I.isHtmlLocked()).toBe(true)
33+
34+
await I.clickClose()
35+
await I.seeNoZagLockEventually()
36+
})
37+
})
38+
})

‎examples/next-ts/pages/scroll-lock/html-scroller.tsx‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import * as dialog from "@zag-js/dialog"
2+
import { Portal, normalizeProps, useMachine } from "@zag-js/react"
3+
import { useEffect } from "react"
4+
5+
export default function Page() {
6+
const service = useMachine(dialog.machine, { id: "1" })
7+
const api = dialog.connect(service, normalizeProps)
8+
9+
// Make <html> establish its own scroll container, like an app-shell layout that scrolls the
10+
// viewport itself rather than relying on <body>'s default overflow propagation.
11+
useEffect(() => {
12+
document.documentElement.style.overflowY = "auto"
13+
return () => {
14+
document.documentElement.style.removeProperty("overflow-y")
15+
}
16+
}, [])
17+
18+
return (
19+
<main>
20+
<p>The dialog's scroll lock should apply to &lt;html&gt;, not &lt;body&gt;, on this page.</p>
21+
22+
<button {...api.getTriggerProps()} data-testid="dialog-trigger">
23+
Open Dialog
24+
</button>
25+
26+
{api.open && (
27+
<Portal>
28+
<div {...api.getBackdropProps()} />
29+
<div {...api.getPositionerProps()}>
30+
<div {...api.getContentProps()}>
31+
<h2 {...api.getTitleProps()}>Dialog</h2>
32+
<p {...api.getDescriptionProps()}>Dialog content</p>
33+
<button {...api.getCloseTriggerProps()} data-testid="dialog-close">
34+
Close
35+
</button>
36+
</div>
37+
</div>
38+
</Portal>
39+
)}
40+
41+
<Portal>
42+
<div data-testid="tall-content" style={{ height: "4000px" }} />
43+
</Portal>
44+
</main>
45+
)
46+
}

‎packages/utilities/remove-scroll/src/index.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getComputedStyle, isIos, setStyleProperty, setStyle } from "@zag-js/dom-query"
1+
import { getComputedStyle, isIos, isOverflowElement, setStyleProperty, setStyle } from "@zag-js/dom-query"
22

33
const LOCK_CLASSNAME = "data-scroll-lock"
44

@@ -22,9 +22,17 @@ function hasStableScrollbarGutter(element: HTMLElement): boolean {
2222
return scrollbarGutter === "stable" || scrollbarGutter?.startsWith("stable ") === true
2323
}
2424

25+
// html scrolls when it overflows, body otherwise (e.g. app-shell layouts where body is
26+
// permanently hidden and an inner region scrolls instead).
27+
function getScrollContainer(doc: Document): HTMLElement {
28+
const { documentElement, body } = doc
29+
return isOverflowElement(documentElement) ? documentElement : body
30+
}
31+
2532
function applyLock(doc: Document): VoidFunction {
2633
const win = doc.defaultView ?? window
2734
const { documentElement, body } = doc
35+
const scroller = getScrollContainer(doc)
2836

2937
// Check if scrollbar-gutter: stable is set on html or body
3038
// If so, the browser already reserves space for the scrollbar
@@ -37,7 +45,7 @@ function applyLock(doc: Document): VoidFunction {
3745
const setScrollbarWidthProperty = () => setStyleProperty(documentElement, "--scrollbar-width", `${scrollbarWidth}px`)
3846
const paddingProperty = getPaddingProperty(documentElement)
3947

40-
const setBodyStyle = () => {
48+
const setScrollerStyle = () => {
4149
// Only add padding if scrollbar-gutter: stable is not set
4250
const styles: Record<string, string> = {
4351
overflow: "hidden",
@@ -47,7 +55,7 @@ function applyLock(doc: Document): VoidFunction {
4755
styles[paddingProperty] = `${scrollbarWidth}px`
4856
}
4957

50-
return setStyle(body, styles)
58+
return setStyle(scroller, styles)
5159
}
5260

5361
// Only iOS doesn't respect `overflow: hidden` on document.body
@@ -79,7 +87,7 @@ function applyLock(doc: Document): VoidFunction {
7987
}
8088
}
8189

82-
const cleanups = [setScrollbarWidthProperty(), isIos() ? setBodyStyleIOS() : setBodyStyle()]
90+
const cleanups = [setScrollbarWidthProperty(), isIos() ? setBodyStyleIOS() : setScrollerStyle()]
8391

8492
return () => {
8593
cleanups.forEach((fn) => fn?.())

‎packages/utilities/remove-scroll/tests/index.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ function resetBody() {
1313
document.body.removeAttribute(LOCK_ATTR)
1414
document.body.removeAttribute("style")
1515
document.documentElement.style.removeProperty("--scrollbar-width")
16+
document.documentElement.style.removeProperty("overflow")
17+
document.documentElement.style.removeProperty("overflow-y")
1618
}
1719

1820
describe("preventBodyScroll", () => {
@@ -148,4 +150,23 @@ describe("preventBodyScroll", () => {
148150
expect(spy).toHaveBeenCalledWith(LOCK_ATTR)
149151
spy.mockRestore()
150152
})
153+
154+
describe("viewport scroller targeting", () => {
155+
test("locks body by default", () => {
156+
const release = preventBodyScroll(document)
157+
expect(document.body.style.overflow).toBe("hidden")
158+
expect(document.documentElement.style.overflow).not.toBe("hidden")
159+
release()
160+
})
161+
162+
test("locks html instead when it establishes its own scroll container (app-shell layout)", () => {
163+
document.documentElement.style.overflowY = "auto"
164+
165+
const release = preventBodyScroll(document)
166+
expect(document.documentElement.style.overflow).toBe("hidden")
167+
expect(document.body.style.overflow).not.toBe("hidden")
168+
169+
release()
170+
})
171+
})
151172
})

‎shared/src/routes.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ export const componentRoutes: ComponentRoute[] = [
6464
{ slug: "column-reverse", title: "Column Reverse" },
6565
],
6666
},
67+
{
68+
slug: "scroll-lock",
69+
label: "Scroll Lock",
70+
examples: [{ slug: "html-scroller", title: "HTML Scroll Container" }],
71+
},
6772
{
6873
slug: "async-list",
6974
label: "Async List",

0 commit comments

Comments
 (0)