fix: measureInWindow calculations for Modal with formSheet presentation#56062
Open
kirillzyusko wants to merge 2 commits intofacebook:mainfrom
Open
fix: measureInWindow calculations for Modal with formSheet presentation#56062kirillzyusko wants to merge 2 commits intofacebook:mainfrom
measureInWindow calculations for Modal with formSheet presentation#56062kirillzyusko wants to merge 2 commits intofacebook:mainfrom
Conversation
2 tasks
kirillzyusko
added a commit
to kirillzyusko/react-native-keyboard-controller
that referenced
this pull request
Mar 12, 2026
## 📜 Description Fixed broken `measureInWindow` measurements in react-native. ## 💡 Motivation and Context The `measureInWindow` works unreliably in Fabric + formSheet Modal (facebook/react-native#56062) or on Android with edge-to-edge enabled (facebook/react-native#56056) Upstream fixes are available, but the problem is that it will require at least RN 0.85+ to work properly. So in this PR we add internal function that is capable of measuring given view. In future this functionality can be removed, but for now this is critical to have it bundled within a package. Fixes #1356 ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### JS - added `viewPositionInWindow` to `types`; - added `viewPositionInWindow` to codegen; - added `viewPositionInWindow` to bindings/module; - use `viewPositionInWindow` instead of `measureInWindow`; - added `viewPositionInWindow` to mocks; ### iOS - implement `viewPositionInWindow`; - make `activeWindow` objc available; ### Android - implement `viewPositionInWindow`; - add `uiManager` and `eventDispatcher` to `ReactContext` extensions; ## 🤔 How Has This Been Tested? Tested manually on iPhone 17 Pro (iOS 26.2, simulator) and Pixel 9 Pro (API 35, emulator). ## 📸 Screenshots (if appropriate): ### Android #### Fabric |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/afb99e51-043f-459c-90e3-7e1eb1e184ed">|<video src="https://github.com/user-attachments/assets/8ea0d593-f7bd-47a5-9343-e2fd85f4af45">| #### Paper |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/702c7d88-755b-45f0-b39f-fe4aaa60ab2e">|<video src="https://github.com/user-attachments/assets/f5346bbd-cc85-4a40-9cc5-9422931b04af">| ### iOS #### Fabric |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/49fb9ac6-15b8-4842-939c-4ec06a4e0c42">|<video src="https://github.com/user-attachments/assets/b559a52a-0a91-48bc-911a-7125ce96e013">| #### Paper |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/c4d234a7-122a-4023-8d86-fad71ed043dc">|<video src="https://github.com/user-attachments/assets/d5d48487-16ad-40c2-b6c6-fea85b563e20">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed --------- Co-authored-by: thomasvo <thomas.vo@openspace.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: kirillzyusko <zyusko.kirik@gmail.com>
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:
Fixed
useWindowDimensionscalculations forModalwithformSheetpresentation.This is only Fabric problem (paper works well because it relies on UIKit implementation).
Problem
The core issue is in how
measureInWindowworks differently between Paper and Fabric.Paper calls iOS native
[view.window convertRect:view.frame fromView:view.superview]which uses UIKit's actual view hierarchy. When aformSheetmodal is presented, the modal's view controller lives in a separateUIWindowor has a known offset from the top of the screen. UIKit'sconvertRect:fromView:correctly accounts for this because it works with the real rendered view positions.Fabric's
measureInWindow(inDOM.cpp) works entirely in the shadow tree, not the native view hierarchy. It callsgetLayoutMetricsFromRootwhich walks up the shadow node ancestors accumulating frame origins. The critical logic is atLayoutableShadowNode.cpp:auto shouldApplyTransformation = (policy.includeTransform && !isRootNode) || (policy.includeViewportOffset && isRootNode);The
includeViewportOffsetflag only applies the root node's transform. For aformSheetmodal, the Y offset from the top of the screen (the ~62pt gap where the modal doesn't cover the full screen) would need to be represented as a transform on the root shadow node. But the modal's screen position is a UIKit concern — it's theUIViewControllerpresentation that positions the modal content on screen, not a shadow tree transform.The key is at
ModalHostViewShadowNode.h:The
ModalHostViewShadowNodehas theRootNodeKindtrait. This means whencomputeRelativeLayoutMetricswalks up from the view, it stops at the modal node (seeLayoutableShadowNode.cpp):Then at the "apply transform" step:
auto shouldApplyTransformation = (policy.includeTransform && !isRootNode) || (policy.includeViewportOffset && isRootNode);For the Modal's root node,
includeViewportOffset && isRootNodeistrue, so it appliesgetTransform(). ButModalHostViewShadowNodedoesn't overridegetTransform()— onlyRootShadowNodedoes (which translates byviewportOffset). The defaultgetTransform()returnsTransform::Identity().So the modal's screen position (the formSheet top inset) is simply never encoded anywhere in the shadow tree.
Solution
The fix would be in
RCTModalHostViewComponentView.mm. InboundsDidChange, when the modal's bounds change, the state is updated with just the size:The fix needs to:
ModalHostViewStateto include aviewportOffset(the modal's origin on screen)boundsDidChangecompute the modal view controller's actual screen position (e.g.,self.viewController.view.frame.originin window coordinates) and pass it as the viewport offsetgetTransform()onModalHostViewShadowNodeto apply the viewport offset from state.This way, when
measureInWindowwalks up to the modal (which is aRootNodeKind),includeViewportOffsettriggersgetTransform(), which now returns the actual screen offset instead of identity.Changelog:
[IOS] [FIXED]
measureInWindowcalculations forModalwithformSheetpresentationTest Plan:
I used this code in RNTester:
Click to expand
If you press on a red rectangle with a fix you will see a correct
y=72(10pt padding in container + 62pt safe area padding):Without a fix the
y=10(which is incorrect).