fix(iOS): wrong height of formSheet in nested stack with fitToContents
#2670
Conversation
This file contains 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
fitToContents fitToContents
Merged
8 tasks
kkafar
added a commit
that referenced
this pull request
Feb 11, 2025
…ew from content wrapper (#2683) ## Description In #2670 I've added a check that logged RN error in case `contentWrapper.reactSuperview` is not a `RNSScreenView`. Now I got reports (but no reproduction :/) from two independent sources that this error is triggered pretty often, which is unexpected from my perspective. I blindly blame view flattening / usage of legacy navigators but can not be sure. This PR is an attempt to patch this behaviour. ## Changes contentWrapper now searches whole view ancestor chain before logging an error. ## Test code and steps to reproduce No reproducer :/ ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: <!-- For adding new props to native-stack --> - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes
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.
Description
Closes #2665
In case when we present
formSheetwith header, the content is nested in another stack -> content wrapper is not mounted directly under presented screen --> we need to forward information of content dimensions from nested content wrapper to the screen withformSheetpresentation.Due to order of updates on Fabric (bottom up assembling of the host tree) first moment when we can look for ancestor screen with
formSheetpresentation is when we move to window...And that is what I did. Now
RNSScreenContentWrapperlooks for appropriate screen to attach to inwillMoveToWindow.There was another problem: any stack along the way impacts desired height of the sheet -> therefore while looking for appropriate screen the wrapper sums up heights of encountered navigation bars and gives that information to the screen.
Important
using
formSheetwith header enabled could lead to flicker (we won't be able to solve it for now, until we are able to trigger react commit & layout synchronously). Maybe we should describe this in types?Changes
☝🏻
Test code and steps to reproduce
TestFormSheet+ setheaderShown: trueonformSheetscreen.Checklist