fix(drawer): tolerate fractional scroll metrics - #36520
Open
Mohd Saif (mohd-saif-1850) wants to merge 1 commit into
Open
fix(drawer): tolerate fractional scroll metrics#36520Mohd Saif (mohd-saif-1850) wants to merge 1 commit into
Mohd Saif (mohd-saif-1850) wants to merge 1 commit into
Conversation
Mohd Saif (mohd-saif-1850)
requested review from
a team and
Victor Genaev (mainframev)
as code owners
August 5, 2026 16:45
| * @param element - HTMLElement to check scroll state of | ||
| */ | ||
| const getScrollState = ({ scrollTop, scrollHeight, clientHeight }: HTMLElement): DrawerScrollState => { | ||
| const epsilon = 1; |
Contributor
There was a problem hiding this comment.
The change is reasonable, browser layout values can be fractional, so strict equality may fail near the bottom.
Main criticisms:
epsilon = 1 is unexplained and could classify the drawer as bottom while still 1px away. Name it descriptively or document why 1px is acceptable.
The top check remains strict. For consistency, consider scrollTop <= epsilon, especially if fractional or slightly negative values can occur.
Prefer comparing the remaining distance
I'd suggest
const scrollTolerance = 1;
if (scrollHeight <= clientHeight) return 'none';
if (scrollTop <= scrollTolerance) return 'top';
if (scrollHeight - clientHeight - scrollTop <= scrollTolerance) return 'bottom';
return 'middle';
Tests should be added to cover fractional values, exact boundaries, and values within/outside the tolerance.
PaulGMardling
requested changes
Aug 6, 2026
Contributor
|
#36333 |
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
Tudor Popa (tudorpopams)
requested review from
Martin Hochel (Hotell) and
Dmytro Kirpa (dmytrokirpa)
and removed request for
Martin Hochel (Hotell) and
Dmytro Kirpa (dmytrokirpa)
August 6, 2026 14:32
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -23,6 +23,7 @@ import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts | |||
| * @param element - HTMLElement to check scroll state of | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Avatar Converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Avatar Converged.badgeMask.normal.chromium.png | 5 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 882 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 16 | Changed |
vr-tests-react-components/ProgressBar converged 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png | 59 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 69 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png | 27 | Changed |
vr-tests-react-components/Skeleton converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Skeleton converged.Opaque Skeleton with rectangle - Dark Mode.default.chromium.png | 9 | Changed |
vr-tests-react-components/TagPicker 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - RTL.disabled input hover.chromium.png | 635 | Changed |
| vr-tests-react-components/TagPicker.disabled - Dark Mode.disabled input hover.chromium.png | 658 | Changed |
There were 2 duplicate changes discarded. Check the build logs for more information.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the scroll-bottom detection in
DrawerBody.Previously, the component checked whether the drawer had reached the bottom using an exact equality comparison: