-
-
Notifications
You must be signed in to change notification settings - Fork 598
feat(Android, Tabs): handle preventing tab change on Android #3350
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
|
|
||
| appearanceCoordinator.updateTabAppearance(this) | ||
|
|
||
| menuItemSelectedViaContainerUpdate = true |
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.
Can we call it in updateSelectedTab instead of updateBottomNavigationViewAppearance? It would be a bit more logical there, or these 2 are not always called together ?
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.
I thought about it as well but I think on Android managing BottomNavigationView and shown screens/fragments is separated and those methods reflect that. Maybe we should think about renaming those methods though.
|
|
||
| // We need to differentiate between user tapping the menu item | ||
| // and update requested from JS. | ||
| private var menuItemSelectedViaContainerUpdate = false |
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.
Am I missing something, or is this supposed to not be triggered when controlledBottomTabs=false, but it is? Also IMO the name is a bit unclear.
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.
On Android, only controlled bottom tabs are currently supported.
I did use viaContainerUpdate name because I guess that there is a possibility to call container update from native code, not only as a result of JS focused tab change. I can maybe reverse the logic and use menuItemSelectedByUser or something like this but I'm not sure.
Description
Allows preventing tab change on Android. Previously, bottom navigation bar would be desynchronized with shown tab screen (menu item would change even if tab screen would not).
Screen_recording_20251028_110806.mp4
Screen_recording_20251028_110431.mp4
Important
Due to changes in this PR, tab change behaves differently when tab screens are heavy.
Screen_recording_20251028_111634.mp4
Screen_recording_20251028_111759.mp4
This matches behavior on iOS but I'm not sure if this is something we want.
Screen.Recording.2025-10-28.at.11.24.49.mov
Closes https://github.com/software-mansion/react-native-screens-labs/issues/561.
Changes
menuItemSelectedViaContainerUpdateto differentiate between user menu item taps and updates from JSmenuItemSelectedViaContainerUpdate, otherwise just send event to JSTest code and steps to reproduce
Run
TestBottomTabs. Apply this patch toBottomTabsContainerto simulate preventing tab change to Tab4:Patch
Try to select tab4.
Checklist