Skip to content
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.

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

bugfix(react-dialog): fix scroll locking issues introduced by a regression #31377

Conversation

bsunderhus
Copy link
Contributor

@bsunderhus bsunderhus commented May 15, 2024

Previous Behavior

  1. In chromium browsers the dialog seem to jump on entering animation
  2. in firefox and safari the body lock mechanism is not working (there's a condition check if the scroll bar is present, that condition is returning false negatives)
    const isScrollbarVisible =
    (targetDocument.defaultView?.innerWidth ?? 0) > targetDocument.documentElement.clientWidth;
    if (!isScrollbarVisible) {
    return;

New Behavior

  1. moves scroll body locking mechanism from useDialog to useDialogSurface to optain access to the animation transitionStatus (solves issue 1.)
  2. adds overflow-y: hidden to body (seems like in html element is not enough
  3. modifies the condition check if the scroll bar is present https://github.com/microsoft/fluentui/pull/31377/files#diff-8f2d1997fb63aa1fca8391cd2ab72b3f776d4d80b89cf2b331321bd11fa60de6R22-R26

Related Issue(s)

@github-actions github-actions bot added this to the April Project Cycle Q1 2024 milestone May 15, 2024
@bsunderhus bsunderhus self-assigned this May 15, 2024
Copy link

codesandbox-ci bot commented May 15, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@fabricteam
Copy link
Collaborator

fabricteam commented May 15, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.139 MB
275.369 kB
1.139 MB
275.438 kB
167 B
69 B
react-dialog
Dialog (including children components)
100.769 kB
30.092 kB
100.192 kB
29.884 kB
-577 B
-208 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
71.55 kB
20.584 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
221.825 kB
62.568 kB
react-components
react-components: FluentProvider & webLightTheme
44.037 kB
14.418 kB
react-portal-compat
PortalCompatProvider
8.39 kB
2.64 kB
🤖 This report was generated against 01a93ee63be8c6120706a227e3e8290766619cce

@fabricteam
Copy link
Collaborator

fabricteam commented May 15, 2024

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 634 604 5000
Button mount 305 282 5000
Field mount 1153 1122 5000
FluentProvider mount 710 708 5000
FluentProviderWithTheme mount 85 87 10
FluentProviderWithTheme virtual-rerender 32 40 10
FluentProviderWithTheme virtual-rerender-with-unmount 72 74 10
MakeStyles mount 850 858 50000
Persona mount 1761 1701 5000
SpinButton mount 1393 1355 5000
SwatchPicker mount 1499 1526 5000

@bsunderhus bsunderhus marked this pull request as ready for review May 15, 2024 08:22
@bsunderhus bsunderhus requested a review from a team as a code owner May 15, 2024 08:22
@bsunderhus bsunderhus force-pushed the react-dialog/bugfix--fix-scroll-lock-issues branch from 50ba485 to 07069fd Compare May 15, 2024 12:16
@bsunderhus bsunderhus merged commit 79b73d9 into microsoft:master May 15, 2024
20 checks passed
@bsunderhus bsunderhus deleted the react-dialog/bugfix--fix-scroll-lock-issues branch May 15, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants