-
-
Notifications
You must be signed in to change notification settings - Fork 597
fix(Android, Stack): incorrect header height values #3402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backward compat can be handled via compatibilityFlags in src/flags.ts. This is how we've done it in react navigation so far.
You add a flag there, denoting that a change has been introduced & then try to read it in react navigation. If it is not present or false, you assume old implementation & if it is present & enabled you assume new one.
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. I've left few remarks.
android/src/main/java/com/swmansion/rnscreens/utils/DimensionUtils.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderHeightUpdateProxy.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt
Show resolved
Hide resolved
kkafar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now. Thank you.
Description
Fixes header height values reported via
onHeaderHeightChangeevent.Fixes #3040.
Fixes #2661.
Note
Due to incorrect values being reported previously, there are some workarounds in
react-navigation'sNativeStackView.native.tsx. Those should be redundant now and we should clean-up that code when this PR is merged. We intend to handle backward compatibility through a flag. PR inreact-navigation: react-navigation/react-navigation#12852.before3402.mp4
after3402.mp4
Changes
TestHeaderHeightto allow testing dynamic changes to the prop andreact-navigation'suseHeaderHeight()hookTest code and steps to reproduce
Run
TestHeaderHeight.Checklist