fix(react): rebind withViewport across windows#36418
Open
Weichenleeeee123 wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes cross-window (e.g., iframe/popup) resizing and measurement behavior for Fluent UI v8 by ensuring withViewport rebinds its resize handling when the root element moves to a different DOM window, and by correcting getRect’s window-selection precedence so an explicitly supplied Window is honored.
Changes:
- Update
withViewportto track the active resize window, and to unregister/re-registerResizeObserveror the fallbackresizelistener when the owner window changes. - Fix
getRectoperator precedence sowin(when provided) takes precedence over the conditional window selection. - Add regression tests covering cross-window observer/listener handoff and explicit-window
getRectbehavior, plus beachball change files for both published packages.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utilities/src/dom/getRect.ts | Fixes win ?? (cond ? ... : ...) grouping so an explicitly supplied window is used correctly. |
| packages/utilities/src/dom/getRect.test.ts | Adds a regression test verifying getRect(frameWindow, frameWindow) measures the supplied window. |
| packages/react/src/utilities/decorators/withViewport.tsx | Rebinds resize handling across window changes and centralizes register/unregister logic. |
| packages/react/src/utilities/decorators/withViewport.test.tsx | Adds tests for ResizeObserver re-registration and fallback resize listener migration across windows. |
| change/@fluentui-utilities-c4e06b96-9dff-4a21-80c7-db044591d953.json | Beachball change entry for the getRect fix. |
| change/@fluentui-react-486db03b-19ec-48a6-91b3-aa08d113d79e.json | Beachball change entry for the withViewport fix. |
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
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.
Summary
withViewportresize handling when its root moves to a different DOM windowResizeObserveror fallback resize listener before registering with the destination windowgetRectso popup viewport measurements use the popup rather than the global windowgetRectRoot cause
withViewportkept its resize handling attached to the window used during mount and did not check whether the root element's owner window changed during an update.After re-registering the observer, popup measurement was also blocked by the precedence of the nullish-coalescing and conditional expressions in
getRect: a supplied popup window was treated as the conditional test and measurement fell back to the global window.Validation
corepack yarn jest packages/react/src/utilities/decorators/withViewport.test.tsx --config packages/react/jest.config.js --runInBand --no-cachecorepack yarn jest packages/utilities/src/dom/getRect.test.ts --config packages/utilities/jest.config.js --runInBand --no-cachecorepack yarn nx run react:lint --skipNxCachecorepack yarn nx run utilities:lint --skipNxCache(passes with two pre-existing warnings in unrelated files)corepack yarn beachball check --branch origin/masterTimePickerassertions which expect English-formatted time; this machine uses thezh-CNlocale (8:30:00/下午5:00:00)Fixes #35437