-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
feat: Update position when target or container dimensions change #30179
feat: Update position when target or container dimensions change #30179
Conversation
Uses a ResizeObserver in the position manager so that a position update is triggered when the dimension of the target or container changes. This should handle most async update scenarios without needing to call `updatePosition` in userland. Fixes #
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 606 | 627 | 5000 | |
Button | mount | 305 | 294 | 5000 | |
Field | mount | 1155 | 1148 | 5000 | |
FluentProvider | mount | 716 | 705 | 5000 | |
FluentProviderWithTheme | mount | 84 | 79 | 10 | |
FluentProviderWithTheme | virtual-rerender | 77 | 69 | 10 | |
FluentProviderWithTheme | virtual-rerender-with-unmount | 86 | 87 | 10 | |
MakeStyles | mount | 846 | 870 | 50000 | |
Persona | mount | 1753 | 1731 | 5000 | |
SpinButton | mount | 1411 | 1381 | 5000 |
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. Latest deployment of this branch, based on commit 489bdc7:
|
📊 Bundle size reportUnchanged fixtures
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: a493617d5c710d78246d315e67fa7eb2d7b6b9f9 (build) |
change/@fluentui-react-positioning-29f6eb38-4af2-40f1-a095-6abef76d64f4.json
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code in this PR LGTM ✅
The idea to have a resize observer for every popup & tooltip looked initially scary, however that statement is not correct: we will have observers only for opened popups (usually 1 or 2) & visible tooltips (should be one on a page).
P.S. Please also consider adding an option to disable this behavior.
* master: (166 commits) Remove v0 dependency from v0 compat package (microsoft#30276) applying package updates Disallow `window` and `document` access for `@fluentui/react` and related packages. (microsoft#30063) Update Rating api and stories (microsoft#30092) TeachingPopover: Minor style changes (microsoft#30270) feat(scripts-gulp): replace lerna with nx (microsoft#30266) ci: remove canary and nightly functionality from northstar (microsoft#30264) List: Re-initialize on mount in React 18. (microsoft#29881) feat(scripts-monorepo): replace lerna/utils with pure nx apis (microsoft#30178) chore: remove react-timepicker-compat-preview (microsoft#30263) applying package updates feat(TimePicker-compat): stable release (microsoft#30217) feat: Implement onPositioningEnd callback (microsoft#30177) applying package updates v8 registerIcons compat (microsoft#30003) Adding Planner, ToDoItem and updated Project filetype icons. Updating FabricCDN url to latest datecode. (microsoft#30079) Scaffolds more Nav components (microsoft#30227) chore: migrate to nx 17.2 (microsoft#30187) applying package updates feat: Update position when target or container dimensions change (microsoft#30179) ...
Uses a ResizeObserver in the position manager so that a position update is triggered when the dimension of the target or container changes. This should handle most async update scenarios without needing to call
updatePosition
in userland.The ResizeObserver will only be created when:
enabled
Practically this means that the ResizeObserver is created only when Popover/Menu/Combobox/Tooltip etc... are open. Just mounting an (unopened) Popover will not create a new ResizeObserver
Fixes #29998