feat: positioning should happen out of React lifecycle #25456

ling1726 · 2022-11-01T08:23:06Z

usePositioning

Positioning is now handled by a new PositioningManager that is a non-react instance. This means that position updates and event listeners can be done without needing to follow react lifecycle.

callback refs are still returned by usePositioning, but internally these callback refs refs are only used to set real refs. This makes the flow of the logic a lot clearer previously the usages of the callback refs could occur before they are even declared.

usePortalMountNode

Also updates usePortalMountNode to create all the attributes of the mount node during the render phase in useMemo to avoid any css variable changes during render time which can cause strange styling bugs due to changes in CSS variables. Since our portals are already broken in react 18 strict mode, we're not making that worse with these changes

Fixes #25432
Fixes #25326

Updates the ref handling done in `usePositioning` so that scroll listeners are only added when both container, target elements are set correctly and the behaviour is `enabled`. This ensures that the browser reflow caused by `getScrollParent` is called as little as possible. Also updates `usePortalMountNode` to create all the attributes of the mount node during the render phase in `useMemo` to avoid any css variable changes during render time which can cause strange styling bugs due to changes in CSS variables. Since our portals are already broken in react 18 strict mode, we're not making that worse with these changes

fabricteam · 2022-11-01T08:37:36Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-combobox` Combobox (including child components)	`75.85 kB` `24.632 kB`	`76.153 kB` `24.665 kB`	`303 B` `33 B`
`react-combobox` Dropdown (including child components)	`75.579 kB` `24.594 kB`	`75.885 kB` `24.635 kB`	`306 B` `41 B`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`189.603 kB` `52.909 kB`	`189.906 kB` `52.963 kB`	`303 B` `54 B`
`react-menu` Menu (including children components)	`116.78 kB` `36.112 kB`	`117.089 kB` `36.188 kB`	`309 B` `76 B`
`react-menu` Menu (including selectable components)	`119.849 kB` `36.635 kB`	`120.158 kB` `36.712 kB`	`309 B` `77 B`
`react-popover` Popover	`103.342 kB` `31.82 kB`	`103.672 kB` `31.871 kB`	`330 B` `51 B`
`react-portal` Portal	`10.628 kB` `3.899 kB`	`10.495 kB` `3.851 kB`	`-133 B` `-48 B`
`react-positioning` usePositioning	`19.724 kB` `7.415 kB`	`19.826 kB` `7.417 kB`	`102 B` `2 B`
`react-tooltip` Tooltip	`41.718 kB` `14.687 kB`	`42.032 kB` `14.739 kB`	`314 B` `52 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-alert` Alert	`84.688 kB` `21.233 kB`
`react-avatar` Avatar	`48.874 kB` `13.864 kB`
`react-avatar` AvatarGroup	`14.996 kB` `6.013 kB`
`react-avatar` AvatarGroupItem	`63.452 kB` `17.959 kB`
`react-components` react-components: Button, FluentProvider & webLightTheme	`62.94 kB` `17.663 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.446 kB` `11.033 kB`
`react-dialog` Dialog (including children components)	`83.147 kB` `24.799 kB`
`react-persona` Persona	`53.992 kB` `15.25 kB`
`react-portal-compat` PortalCompatProvider	`5.857 kB` `1.978 kB`

_{🤖 This report was generated against bfc4a96dc4e1f1b656f8c9f0edd06b1005fc2284}

size-auditor · 2022-11-01T08:38:19Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: bfc4a96dc4e1f1b656f8c9f0edd06b1005fc2284 (build)

fabricteam · 2022-11-01T08:38:45Z

Perf Analysis (`@fluentui/react-components`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1315	1309	5000
Button	mount	902	949	5000
FluentProvider	mount	1594	1563	5000
FluentProviderWithTheme	mount	633	632	10
FluentProviderWithTheme	virtual-rerender	525	597	10
FluentProviderWithTheme	virtual-rerender-with-unmount	634	560	10
MakeStyles	mount	1775	1859	50000
SpinButton	mount	2337	2527	5000

codesandbox-ci · 2022-11-01T08:39:41Z

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 96ad42a:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration
Tooltip and computed style	Issue #25326
inspiring-chihiro-w571tp	Issue #25432

packages/react-components/react-positioning/src/utils/writeContainerupdates.ts

change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json

…63e6a47f5.json

packages/react-components/react-portal/src/components/Portal/usePortalMountNode.ts

packages/react-components/react-positioning/etc/react-positioning.api.md

packages/react-components/react-positioning/src/utils/writeContainerupdates.ts

packages/react-components/react-positioning/src/createPositionManager.ts

layershifter · 2022-11-01T12:31:31Z

packages/react-components/react-positioning/src/usePositioning.ts

+    if (managerRef.current) {
+      managerRef.current.dispose();
    }


Suggested change

if (managerRef.current) {

managerRef.current.dispose();

}

managerRef.current?.dispose();

nit

packages/react-components/react-positioning/src/usePositioning.ts

…ling1726/fluentui into feat/use-positioning-ref-update

* master: (23 commits) fix(docsite-v9): move theme picker under component title so it can be visible on embedded pages (microsoft#25385) applying package updates feat: Implement child render function for DataGrid rows (microsoft#25476) fix(useTable): sort should adapt to enhanced row types (microsoft#25487) applying package updates feat: positioning should happen out of React lifecycle (microsoft#25456) applying package updates chore(react-link): migrate to new package structure (microsoft#25471) chore(react-input): migrate to new package structure (microsoft#25469) fix glob pattern for syncpack (microsoft#25465) chore(react-label): migrate to new package structure (microsoft#25470) chore: bump Griffel to latest (microsoft#25412) chore: add few small toolbar improvements (microsoft#25468) docs: fix small typos (microsoft#25464) chore: fix dependencies in @fluentui/react-toolbar (microsoft#25466) feat: v0 menu style migration from v9 (microsoft#25012) applying package updates Update List to render children on first render() call (microsoft#25331) Scaffold react-skeleton package (microsoft#25435) fix(public-docsite): Changing crossplatform urls to use 'cross' instead of 'crossplatform' (microsoft#25437) ...

* feat: Updates ref update handling in `usePositioning` Updates the ref handling done in `usePositioning` so that scroll listeners are only added when both container, target elements are set correctly and the behaviour is `enabled`. This ensures that the browser reflow caused by `getScrollParent` is called as little as possible. Also updates `usePortalMountNode` to create all the attributes of the mount node during the render phase in `useMemo` to avoid any css variable changes during render time which can cause strange styling bugs due to changes in CSS variables. Since our portals are already broken in react 18 strict mode, we're not making that worse with these changes * revert button changes * changefiles * update md * fix ssr * update * fix blurriness * Update change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json * document usePortalMountNode memo * fix breaking api * rename to targetWindow * createPositionManager is internal * short circuit * update check to include override target * remove useless improt * revert blurriness change * fix position fixed and effect

msft-fluent-ui-bot assigned smhigley Nov 1, 2022

github-actions bot added this to the October Project Cycle Q4 2022 milestone Nov 1, 2022

ling1726 added 3 commits November 1, 2022 09:23

revert button changes

3cad896

changefiles

0e498a7

update md

16af62c

ling1726 added 2 commits November 1, 2022 09:44

fix ssr

3b60653

update

a4db4d9

ling1726 changed the title ~~feat: Updates ref update handling in usePositioning~~ feat: positioning should happen out of React lifecycle Nov 1, 2022

fix blurriness

92e4f17

ling1726 commented Nov 1, 2022

View reviewed changes

packages/react-components/react-positioning/src/utils/writeContainerupdates.ts Outdated Show resolved Hide resolved

msft-fluent-ui-bot added the Component: Menu label Nov 1, 2022

ling1726 commented Nov 1, 2022

View reviewed changes

change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab563e6a47f5.json Outdated Show resolved Hide resolved

Update change/@fluentui-react-positioning-b6d61cdf-e434-43ff-a859-ab5…

c211a6d

…63e6a47f5.json

ling1726 marked this pull request as ready for review November 1, 2022 12:26

ling1726 requested a review from a team as a code owner November 1, 2022 12:26

layershifter reviewed Nov 1, 2022

View reviewed changes

ling1726 added 9 commits November 1, 2022 22:14

document usePortalMountNode memo

91cca72

fix breaking api

f191e8b

rename to targetWindow

22160da

createPositionManager is internal

6251c90

short circuit

5dd2379

update check to include override target

2c1605b

Merge branch 'feat/use-positioning-ref-update' of https://github.com/…

6669e93

…ling1726/fluentui into feat/use-positioning-ref-update

remove useless improt

d9a2d53

revert blurriness change

294aa77

fix position fixed and effect

96ad42a

ling1726 requested a review from layershifter November 2, 2022 11:17

layershifter approved these changes Nov 2, 2022

View reviewed changes

ling1726 merged commit 393a7b1 into microsoft:master Nov 2, 2022

layershifter mentioned this pull request Nov 22, 2022

Wrap toggleScrollListener calls in requestAnimationFrame callback to prevent forced style recalculations #25328

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: positioning should happen out of React lifecycle #25456

feat: positioning should happen out of React lifecycle #25456

ling1726 commented Nov 1, 2022 •

edited

Loading

fabricteam commented Nov 1, 2022 •

edited

Loading

size-auditor bot commented Nov 1, 2022 •

edited

Loading

fabricteam commented Nov 1, 2022 •

edited

Loading

codesandbox-ci bot commented Nov 1, 2022 •

edited

Loading

layershifter Nov 1, 2022

feat: positioning should happen out of React lifecycle #25456

feat: positioning should happen out of React lifecycle #25456

Conversation

ling1726 commented Nov 1, 2022 • edited Loading

usePositioning

usePortalMountNode

fabricteam commented Nov 1, 2022 • edited Loading

📊 Bundle size report

size-auditor bot commented Nov 1, 2022 • edited Loading

Asset size changes

fabricteam commented Nov 1, 2022 • edited Loading

Perf Analysis (@fluentui/react-components)

codesandbox-ci bot commented Nov 1, 2022 • edited Loading

layershifter Nov 1, 2022

Choose a reason for hiding this comment

ling1726 commented Nov 1, 2022 •

edited

Loading

fabricteam commented Nov 1, 2022 •

edited

Loading

size-auditor bot commented Nov 1, 2022 •

edited

Loading

fabricteam commented Nov 1, 2022 •

edited

Loading

Perf Analysis (`@fluentui/react-components`)

codesandbox-ci bot commented Nov 1, 2022 •

edited

Loading