From ecdafe131b4885c10e1afc8a117f0e8c0fde6e56 Mon Sep 17 00:00:00 2001 From: Frank Nambeh Date: Tue, 1 Sep 2026 15:22:57 -0500 Subject: [PATCH] fix(react-drawer): report bottom scroll state at fractional browser zoom `getScrollState` compared scroll offsets for exact equality, but browsers round `clientHeight` and `scrollHeight` to integers while `scrollTop` stays fractional. At non-integer browser zoom a fully scrolled DrawerBody therefore reported `middle` instead of `bottom`, which kept the divider above NavDrawerFooter visible. The same rounding can make a non-scrollable body report a pixel of overflow it cannot actually scroll, reporting `top` and showing the divider on a drawer with nothing to scroll. Compare both with a tolerance instead. Measured against Chromium across 31 zoom levels from 100% to 250%, the bottom offset falls up to 1.44px short of the computed maximum, and phantom overflow is at most 1px. Behaviour at 100% zoom is unchanged. Fixes #36331 Co-authored-by: Cursor --- ...-f4a6631d-4008-4940-935a-b7f13c49dd23.json | 7 ++ .../components/DrawerBody/DrawerBody.test.tsx | 93 +++++++++++++++++++ .../components/DrawerBody/useDrawerBody.ts | 20 +++- 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 change/@fluentui-react-drawer-f4a6631d-4008-4940-935a-b7f13c49dd23.json diff --git a/change/@fluentui-react-drawer-f4a6631d-4008-4940-935a-b7f13c49dd23.json b/change/@fluentui-react-drawer-f4a6631d-4008-4940-935a-b7f13c49dd23.json new file mode 100644 index 00000000000000..18305f51f4b4c9 --- /dev/null +++ b/change/@fluentui-react-drawer-f4a6631d-4008-4940-935a-b7f13c49dd23.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: compare DrawerBody scroll positions with a tolerance so the footer divider is hidden when scrolled to the bottom at fractional browser zoom", + "packageName": "@fluentui/react-drawer", + "email": "fnambeh@gsumail.gram.edu", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-drawer/library/src/components/DrawerBody/DrawerBody.test.tsx b/packages/react-components/react-drawer/library/src/components/DrawerBody/DrawerBody.test.tsx index 5b2a912d41d320..44faf81fa2e13b 100644 --- a/packages/react-components/react-drawer/library/src/components/DrawerBody/DrawerBody.test.tsx +++ b/packages/react-components/react-drawer/library/src/components/DrawerBody/DrawerBody.test.tsx @@ -1,8 +1,39 @@ import * as React from 'react'; import { render } from '@testing-library/react'; import { DrawerBody } from './DrawerBody'; +import { DrawerProvider, useDrawerContextValue } from '../../contexts'; import { isConformant } from '../../testing/isConformant'; +type ScrollMetrics = { + scrollTop: number; + clientHeight: number; + scrollHeight: number; +}; + +/** + * jsdom reports 0 for every layout measurement, so the values `getScrollState` reads are stubbed + * on the prototype. Fractional values reproduce what browsers report when zoom is not a whole + * number: `scrollTop` keeps its fraction while `clientHeight` and `scrollHeight` are rounded. + */ +const scrollMetrics: ScrollMetrics = { scrollTop: 0, clientHeight: 0, scrollHeight: 0 }; + +const ScrollStateHarness: React.FC = () => { + const contextValue = useDrawerContextValue(); + + return ( + + Content +
{contextValue.scrollState}
+ + ); +}; + +const renderWithScrollMetrics = (metrics: ScrollMetrics): string | null => { + Object.assign(scrollMetrics, metrics); + + return render().getByTestId('scroll-state').textContent; +}; + describe('DrawerBody', () => { isConformant({ Component: DrawerBody, @@ -21,4 +52,66 @@ describe('DrawerBody', () => { `); }); + + describe('scroll state', () => { + const originalDescriptors = new Map(); + + beforeAll(() => { + (Object.keys(scrollMetrics) as (keyof ScrollMetrics)[]).forEach(key => { + originalDescriptors.set(key, Object.getOwnPropertyDescriptor(HTMLElement.prototype, key)); + Object.defineProperty(HTMLElement.prototype, key, { + configurable: true, + get: () => scrollMetrics[key], + }); + }); + }); + + afterAll(() => { + originalDescriptors.forEach((descriptor, key) => { + if (descriptor) { + Object.defineProperty(HTMLElement.prototype, key, descriptor); + } else { + // These live on Element.prototype, so removing the stub restores the inherited accessor. + Reflect.deleteProperty(HTMLElement.prototype, key); + } + }); + }); + + it('reports "none" when the content fits', () => { + expect(renderWithScrollMetrics({ scrollTop: 0, clientHeight: 300, scrollHeight: 300 })).toBe('none'); + }); + + it('reports "top" when scrollable and not scrolled', () => { + expect(renderWithScrollMetrics({ scrollTop: 0, clientHeight: 300, scrollHeight: 500 })).toBe('top'); + }); + + it('reports "middle" when scrolled between the ends', () => { + expect(renderWithScrollMetrics({ scrollTop: 100, clientHeight: 300, scrollHeight: 500 })).toBe('middle'); + }); + + it('reports "bottom" when scrolled to the end', () => { + expect(renderWithScrollMetrics({ scrollTop: 200, clientHeight: 300, scrollHeight: 500 })).toBe('bottom'); + }); + + // Regression: https://github.com/microsoft/fluentui/issues/36331 + // At fractional browser zoom the scroll offset stops short of `scrollHeight - clientHeight`, + // which used to leave the state at 'middle' and kept the DrawerFooter divider visible. + // 198.5652 is the widest gap measured in Chromium, at 115% zoom. + const fractionalOffsets: Array<[string, number]> = [ + ['110% zoom', 199.0909], + ['115% zoom', 198.5652], + ['125% zoom', 199.8], + ['150% zoom', 199.6667], + ]; + + it.each(fractionalOffsets)('reports "bottom" when scrolled to the end at %s', (_zoom, scrollTop) => { + expect(renderWithScrollMetrics({ scrollTop, clientHeight: 300, scrollHeight: 500 })).toBe('bottom'); + }); + + // Fractional layout can round `scrollHeight` a pixel above `clientHeight` on an element that + // cannot actually be scrolled, which used to report 'top' and show the divider. + it('reports "none" when overflow is within rounding error of the client height', () => { + expect(renderWithScrollMetrics({ scrollTop: 0, clientHeight: 300, scrollHeight: 301 })).toBe('none'); + }); + }); }); diff --git a/packages/react-components/react-drawer/library/src/components/DrawerBody/useDrawerBody.ts b/packages/react-components/react-drawer/library/src/components/DrawerBody/useDrawerBody.ts index 881fc003aa0066..d8a1443a10418b 100644 --- a/packages/react-components/react-drawer/library/src/components/DrawerBody/useDrawerBody.ts +++ b/packages/react-components/react-drawer/library/src/components/DrawerBody/useDrawerBody.ts @@ -16,6 +16,22 @@ import type { DrawerScrollState } from '../../shared/DrawerBase.types'; import type { DrawerBodyProps, DrawerBodyState } from './DrawerBody.types'; import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; +/** + * `clientHeight` and `scrollHeight` are rounded to integers while `scrollTop` is fractional, so + * scroll positions have to be compared with a tolerance instead of for exact equality. Without it + * a fully scrolled body reports `middle` whenever the browser zoom is not a whole number. + * + * A non-scrollable element can report up to 1px of overflow that it cannot actually scroll, and + * both values are integers, so a single pixel is enough here. + */ +const OVERFLOW_TOLERANCE = 1; + +/** + * A fully scrolled element can sit short of its own maximum scroll offset because `scrollTop` is + * fractional while the values it is compared against are rounded. + */ +const SCROLL_BOTTOM_TOLERANCE = 2; + /** * Get the current scroll state of the DrawerBody. * @@ -23,7 +39,7 @@ import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts * @param element - HTMLElement to check scroll state of */ const getScrollState = ({ scrollTop, scrollHeight, clientHeight }: HTMLElement): DrawerScrollState => { - if (scrollHeight <= clientHeight) { + if (scrollHeight - clientHeight <= OVERFLOW_TOLERANCE) { return 'none'; } @@ -31,7 +47,7 @@ const getScrollState = ({ scrollTop, scrollHeight, clientHeight }: HTMLElement): return 'top'; } - if (scrollTop + clientHeight === scrollHeight) { + if (scrollHeight - clientHeight - scrollTop <= SCROLL_BOTTOM_TOLERANCE) { return 'bottom'; }