react-native-screens
react-native-screens copied to clipboard
fix(new arch): change how screen size is calculated
Description
During the migration of Expensify App to the new architecture, we found a bug where the screen size consistently doesn't get updated to the correct one when navigating between screens while the keyboard is opened. It seems like updateScreenSizeFabric
function was the problem, as removing it solved the problem (we're using JS stack).
The problem this method was supposed to fix is that yoga doesn't know about the header's existence so the screen would be too big (by the height of the header) by setting the size of the screen to match one from the native layout. I think that the issue with the keyboard could be caused by reusing the values from yoga during the native layout, keeping the wrong screen dimensions.
In https://github.com/software-mansion/react-native-screens/pull/2028 @WoLewicki added the header height to the state of the screen, which gave me an idea: we could let yoga layout the screen, giving a result with the wrong height and cut it by the size of the header we now have access to.
For now it's done using padding, which means that calling measure
on screen would return wrong height, other than that I'm not aware of situations where this approach could break. The better approach would be to apply this change during the layout pass, but overriding layoutmetrics
didn't really work for me.
Note: there are probably parts of this PR that are better suited to be in https://github.com/software-mansion/react-native-screens/pull/2028. Once @WoLewicki is back we'll need to discuss it.
Changes
- Changed how screen size is calculated on the new architecture
Screenshots / GIFs
Before
https://github.com/software-mansion/react-native-screens/assets/21055725/03e68f4a-8ae0-47fc-ba02-e21676d97196
After
https://github.com/software-mansion/react-native-screens/assets/21055725/d7d2ced3-7aa7-4326-9d72-f5b3821ec1bd
Test code and steps to reproduce
The only place I could reproduce this issue is on the Expensify App with the new arch enabled in release mode on Android. This is the PR: https://github.com/Expensify/App/pull/13767.
Checklist
- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Updated documentation:
- [ ] 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