-
-
Notifications
You must be signed in to change notification settings - Fork 607
feat(Tabs): special effects refactor #3440
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
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.
Looks good! I have few questions to fully comprehend the changes.
|
|
||
| // All events for a given view can be coalesced. | ||
| override fun getCoalescingKey(): Short = 0 | ||
| override fun getCoalescingKey(): Short = (tabNumber * 10 + if (repeatedSelectionHandledBySpecialEffect) 1 else 0).toShort() |
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.
What's the reasoning here? Why different key when repeatedSelectionHandledBySpecialEffect?
FYI: I don't think this works anymore, as recently discovered by @t0maboro.
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.
If the user taps currently selected tab 2 times and e.g. scroll to top effect can run, we should send [(tabKey, true), (tabKey, false)]. If I understand correctly, coalescing key is used to combine events to prevent spamming them. If those 2 events were combined to only one, we would lose some information that somebody can rely on (e.g. running some action at the same time as special effect or after special effect).
FYI: I don't think this works anymore, as recently discovered by @t0maboro.
Just wanted to make sure that it works correctly when it gets fixed 😅
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.
This convinces me. I think this deserves code comment. Let's make one, please.
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.
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.
Okay, looks good! Thank you!
## Description Refactors special effects on both platforms: - fixes disabling `popToRoot` on iOS 26, - fixes special effects running on regular (not repeated) tab change on Android, - adds `repeatedSelectionHandledBySpecialEffect` to `NativeFocusChangeEvent` on both platforms (now the event is sent on every tab press with appropriate flag value), - adds support for `specialEffects` on Paper+iOS. Closes software-mansion/react-native-screens-labs#579. Closes software-mansion/react-native-screens-labs#574. ### Recordings Pay attention to logs. #### Android https://github.com/user-attachments/assets/54887d06-f6fa-4bb9-a9bd-2642a8af31b2 #### iOS https://github.com/user-attachments/assets/c06505dc-e4e6-44b9-90f3-5d18c3345c01 ### Native pop to root on iOS 26 UIKit natively has "pop to root" effect in `UITabBarController` but prior to iOS 26, it worked only when `UINavigationController` was direct child of `UITabBarController`. In `screens`, `RNSTabBarController` has `RNSTabsScreenViewController` children view controllers and `RNSScreenStack` can be a child VC of `RNSTabsScreenViewController`. This prevents native effect from working on iOS versions prior to 26. Starting from iOS 26, `UITabBarController` also detects nested `UINavigationController`s. This interaction happens when we return `true` from `tabBarController:shouldSelectViewController:` delegate method. This would work for natively-driven tabs but for controlled tabs, we need to return `false` to prevent tab change on the native side -> that's why we decided to use only our own implementation and disable native effect by always returning `false` from `tabBarController:shouldSelectViewController:` on repeated tab selection. ## Changes - handle `specialEffects` prop on iOS, Paper, - add `repeatedSelectionHandledBySpecialEffect` to `NativeFocusChangeEvent` - update docs (move special effects and freeze to general section, clean up) - return `false` from `tabBarController:(UITabBarController *)tabBarController shouldSelectViewController:(UIViewController *)viewController` on repeated tab selection on iOS (disables native pop to root interaction which is available starting from iOS 26) - prevent running `onItemSelectedListener` after menu selection on Android (it runs on click for the first time and it would run for the second time after update from JS which would activate special effect erroneously) ## Test code and steps to reproduce Run `TestBottomTabs`. Test repeated selection with different special effects enabled on `Tab4`. Make sure that special effects don't run when changing from other tab to `Tab4`. ## Checklist - [x] Included code example that can be used to test this change - [x] Updated TS types - [x] Updated documentation - [x] Ensured that CI passes
Description
Refactors special effects on both platforms:
popToRooton iOS 26,repeatedSelectionHandledBySpecialEffecttoNativeFocusChangeEventon both platforms (now the event is sent on every tab press with appropriate flag value),specialEffectson Paper+iOS.Closes https://github.com/software-mansion/react-native-screens-labs/issues/579.
Closes https://github.com/software-mansion/react-native-screens-labs/issues/574.
Recordings
Pay attention to logs.
Android
Screen.Recording.2025-12-01.at.11.03.54.mov
iOS
Screen.Recording.2025-12-01.at.11.06.23.mov
Native pop to root on iOS 26
UIKit natively has "pop to root" effect in
UITabBarControllerbut prior to iOS 26, it worked only whenUINavigationControllerwas direct child ofUITabBarController. Inscreens,RNSTabBarControllerhasRNSTabsScreenViewControllerchildren view controllers andRNSScreenStackcan be a child VC ofRNSTabsScreenViewController. This prevents native effect from working on iOS versions prior to 26.Starting from iOS 26,
UITabBarControlleralso detects nestedUINavigationControllers. This interaction happens when we returntruefromtabBarController:shouldSelectViewController:delegate method. This would work for natively-driven tabs but for controlled tabs, we need to returnfalseto prevent tab change on the native side -> that's why we decided to use only our own implementation and disable native effect by always returningfalsefromtabBarController:shouldSelectViewController:on repeated tab selection.Changes
specialEffectsprop on iOS, Paper,repeatedSelectionHandledBySpecialEffecttoNativeFocusChangeEventfalsefromtabBarController:(UITabBarController *)tabBarController shouldSelectViewController:(UIViewController *)viewControlleron repeated tab selection on iOS (disables native pop to root interaction which is available starting from iOS 26)onItemSelectedListenerafter menu selection on Android (it runs on click for the first time and it would run for the second time after update from JS which would activate special effect erroneously)Test code and steps to reproduce
Run
TestBottomTabs. Test repeated selection with different special effects enabled onTab4. Make sure that special effects don't run when changing from other tab toTab4.Checklist