Skip to content

fix(react-headless-components-preview): reserve the scrollbar gutter while a dialog locks scroll #36666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ray Knight (ArrayKnight) wants to merge 4 commits into
base: master
Choose a base branch
from
+155 −7
Open

fix(react-headless-components-preview): reserve the scrollbar gutter while a dialog locks scroll #36666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26361fb
fix(react-headless-components-preview): reserve the scrollbar gutter …
ArrayKnight Aug 25, 2026
7d77229
chore: add change file
ArrayKnight Aug 31, 2026
24d31e5
fix(react-headless-components-preview): preserve host scrollbar gutters
ArrayKnight Sep 7, 2026
bffa278
test(react-headless-components-preview): verify scrollbar gutter rest…
ArrayKnight Sep 8, 2026
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
7 changes: 7 additions & 0 deletions change/@fluentui-react-headless-components-preview-4e33a94a-d467-47f7-87d9-2714995d6202.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: reserve the scrollbar gutter while a modal dialog locks document scroll, so opening a dialog no longer shifts the page",
"packageName": "@fluentui/react-headless-components-preview",
"email": "array.knight@gmail.com",
"dependentChangeType": "patch"
}
123 changes: 123 additions & 0 deletions ...omponents/react-headless-components-preview/library/src/components/Dialog/Dialog.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,129 @@ describe('Dialog', () => {
expect(dialog).not.toHaveAttribute('aria-labelledby');
});

describe('scroll lock', () => {
// jsdom reports clientWidth 0, which would read as a scrollbar on every page.
const setScrollbarWidth = (width: number) =>
Object.defineProperty(document.documentElement, 'clientWidth', {
configurable: true,
value: window.innerWidth - width,
});

afterEach(() => {
Reflect.deleteProperty(document.documentElement, 'clientWidth');
document.documentElement.style.removeProperty('scrollbar-gutter');
document.body.style.removeProperty('overflow');
});

const renderModal = () =>
render(
<Dialog unmountOnClose={false}>
<DialogTrigger>
<button>Open dialog</button>
</DialogTrigger>
<DialogSurface>
<DialogTitle>Dialog title</DialogTitle>
<DialogActions>
<DialogTrigger>
<button>Close dialog</button>
</DialogTrigger>
</DialogActions>
</DialogSurface>
</Dialog>,
);

it('reserves the scrollbar gutter while a modal holds the lock', () => {
setScrollbarWidth(15);
const result = renderModal();

fireEvent.click(result.getByRole('button', { name: 'Open dialog' }));

expect(document.body.style.overflow).toBe('visible clip');
// On <html>, not <body>: scrollbar-gutter does not propagate to the viewport.
expect(document.documentElement.style.scrollbarGutter).toBe('stable');

fireEvent.click(result.getByRole('button', { name: 'Close dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe('');
});

it('reserves nothing when the scrollbar takes no layout width', () => {
setScrollbarWidth(0);
const result = renderModal();

fireEvent.click(result.getByRole('button', { name: 'Open dialog' }));

expect(document.body.style.overflow).toBe('visible clip');
expect(document.documentElement.style.scrollbarGutter).toBe('');
});

it('restores an inline auto gutter after the lock replaces it', () => {
setScrollbarWidth(15);
document.documentElement.style.scrollbarGutter = 'auto';
const result = renderModal();

fireEvent.click(result.getByRole('button', { name: 'Open dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe('stable');

fireEvent.click(result.getByRole('button', { name: 'Close dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe('auto');
});

it.each(['stable', 'stable both-edges'])('preserves an inline %s gutter during and after the lock', gutter => {
setScrollbarWidth(15);
document.documentElement.style.scrollbarGutter = gutter;
const result = renderModal();

fireEvent.click(result.getByRole('button', { name: 'Open dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe(gutter);

fireEvent.click(result.getByRole('button', { name: 'Close dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe(gutter);
});

it('preserves a stable both-edges gutter supplied by a stylesheet', () => {
setScrollbarWidth(15);
const stylesheet = document.createElement('style');
stylesheet.textContent = 'html { scrollbar-gutter: stable both-edges; }';
document.head.appendChild(stylesheet);
try {
const result = renderModal();
expect(window.getComputedStyle(document.documentElement).scrollbarGutter).toBe('stable both-edges');

fireEvent.click(result.getByRole('button', { name: 'Open dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe('');
expect(window.getComputedStyle(document.documentElement).scrollbarGutter).toBe('stable both-edges');

fireEvent.click(result.getByRole('button', { name: 'Close dialog' }));

expect(document.documentElement.style.scrollbarGutter).toBe('');
expect(window.getComputedStyle(document.documentElement).scrollbarGutter).toBe('stable both-edges');
} finally {
stylesheet.remove();
}
});

it('leaves a non-modal dialog out of the lock entirely', () => {
setScrollbarWidth(15);
const result = render(
<Dialog defaultOpen modalType="non-modal">
<DialogSurface>
<DialogTitle>Non-modal title</DialogTitle>
</DialogSurface>
</Dialog>,
);

expect(result.container.querySelector('dialog')).toHaveAttribute('data-open');
expect(document.body.style.overflow).toBe('');
expect(document.documentElement.style.scrollbarGutter).toBe('');
});
});

it('keeps dialog mounted after close when unmountOnClose is false', () => {
const result = render(
<Dialog unmountOnClose={false}>
Expand Down
32 changes: 25 additions & 7 deletions ...omponents/react-headless-components-preview/library/src/components/Dialog/utils/scroll.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
type ScrollLockState = {
lockCount: number;
previousBodyOverflow: string;
previousScrollbarGutter: string;
};

const scrollLockStateByDocument = new WeakMap<Document, ScrollLockState>();

/**
* Prevents background scrolling while a modal/alert dialog is open by applying
* `overflow: hidden` to `<body>`. The `<html>` element is intentionally left
* untouched so host-application styles on the document element are preserved.
* `overflow: visible clip` to `<body>`, and reserves the space the page scrollbar was
* occupying so nothing on the page moves sideways as it disappears.
*
* The gutter has to be reserved on `<html>`: `scrollbar-gutter` does not propagate
* from `<body>` to the viewport the way `overflow` does, so spelling it on `<body>`
* reserves nothing. It is written only when the scrollbar actually takes layout
* width, because `stable` otherwise reserves a gutter the page never had.
*
* Nested modal dialogs share a single lock via a reference count.
*/
Expand All @@ -19,18 +25,29 @@ export function lockDocumentScroll(targetDocument: Document): void {
return;
}

const { body, documentElement } = targetDocument;
// Read the scrollbar's layout width before the lock takes it away. Overlay
// scrollbars and unscrollable pages both measure 0, and both want no gutter.
const scrollbarWidth = (targetDocument.defaultView?.innerWidth ?? 0) - documentElement.clientWidth;

@dmytrokirpa Dmytro Kirpa (dmytrokirpa) Sep 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we use useApplyScrollbarWidth or useScrollbarWidth hooks from @fluentui/react-utilities?

@ArrayKnight Ray Knight (ArrayKnight) Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at both hooks before settling on this shape — they solve a different half of the problem, so I kept the native gutter, but happy to switch if you prefer the tradeoff:

  • This fix reserves the gutter natively (scrollbar-gutter: stable on <html>) instead of compensating with a measured pixel width. Its only measurement, innerWidth - documentElement.clientWidth, asks whether this page, right now has a layout-consuming scrollbar.
  • useScrollbarWidth/useApplyScrollbarWidth measure a probe element (measureScrollbarWidth), which reports the UA's classic scrollbar width even when the page itself doesn't scroll — so on an unscrollable page a width/padding compensation would introduce the very shift this PR removes, and we'd still need the current-viewport check on top.
  • useApplyScrollbarWidth is a mount-only ref callback (it writes ${width}px on attach and early-returns on detach), so it can't express the lock's restore semantics — previous inline values plus the refcount for nested modals — and there's no natural ref to hand it for document.documentElement.

Adopting them would replace only the measurement while keeping all the lock/restore code, and would trade native reservation for pixel compensation. If you'd rather standardize on the shared utility regardless, I'm glad to rework it that way.

const scrollbarGutter = targetDocument.defaultView?.getComputedStyle(documentElement).scrollbarGutter;
const hasStableGutter = scrollbarGutter?.split(/\s+/).includes('stable');

scrollLockStateByDocument.set(targetDocument, {
lockCount: 1,
previousBodyOverflow: targetDocument.body.style.overflow,
previousBodyOverflow: body.style.overflow,
previousScrollbarGutter: documentElement.style.scrollbarGutter,
});

targetDocument.body.style.overflow = 'visible clip';
body.style.overflow = 'visible clip';
if (scrollbarWidth > 0 && !hasStableGutter) {
documentElement.style.scrollbarGutter = 'stable';

@ArrayKnight Ray Knight (ArrayKnight) Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed: the lock now reads the root's computed scrollbar-gutter before changing overflow and installs the stable fallback only if a stable gutter is absent. Existing stable and stable both-edges values are preserved while locked and after unlock, including a value supplied by a stylesheet.

The tests use valid CSS values and assert both phases. Temporarily restoring the unconditional fallback fails the inline stable both-edges and stylesheet tests. All 18 Dialog tests and package type-check pass; lint passes with one existing unrelated ToastTitle warning. I also corrected the mocked clientWidth cleanup to use Reflect.deleteProperty, resolving its TypeScript readonly-property error.

}
}

/**
* Restores the document's scroll behavior by reverting the `overflow` style
* on the `<body>` element to its previous value. This function is typically
* called when a modal/alert dialog is closed.
* Restores the document's scroll behavior by reverting the `overflow` style on the
* `<body>` element and the reserved scrollbar gutter on `<html>` to their previous
* values. This function is typically called when a modal/alert dialog is closed.
*/
export function unlockDocumentScroll(targetDocument: Document): void {
const state = scrollLockStateByDocument.get(targetDocument);
Expand All @@ -44,5 +61,6 @@ export function unlockDocumentScroll(targetDocument: Document): void {
}

targetDocument.body.style.overflow = state.previousBodyOverflow;
targetDocument.documentElement.style.scrollbarGutter = state.previousScrollbarGutter;
scrollLockStateByDocument.delete(targetDocument);
}