fix: updateSelectedTab removing unowned fragments#3608
fix: updateSelectedTab removing unowned fragments#3608kkafar merged 1 commit intosoftware-mansion:mainfrom
updateSelectedTab removing unowned fragments#3608Conversation
|
|
||
| check(requireFragmentManager.fragments.size <= 1) { "[RNScreens] There can be only a single focused tab" } | ||
| val oldFocusedTab = requireFragmentManager.fragments.firstOrNull() | ||
| val tabFragments = requireFragmentManager.fragments.filterIsInstance<TabScreenFragment>() |
There was a problem hiding this comment.
overall looks good, but I'm wondering whether we could use tabScreenFragments and detect it by isFocusedTab, then we'd always operate only on our fragments (on the other hand, RN is becoming the source of truth then, instead of native state, so this might not be valid), cc-ing @kkafar as he has better context for the tabs impl. on Android
There was a problem hiding this comment.
not really. tabScreenFragments do hold fragments for all rendered screens, possibly many, while there should be only a single TabScreenFragment of ours attached to the fragment manager. And this is what these assertions are for.
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
We'll have potentially similar problems in stack.
Here I guess it is fine right now as a quick-patch.
|
|
||
| check(requireFragmentManager.fragments.size <= 1) { "[RNScreens] There can be only a single focused tab" } | ||
| val oldFocusedTab = requireFragmentManager.fragments.firstOrNull() | ||
| val tabFragments = requireFragmentManager.fragments.filterIsInstance<TabScreenFragment>() |
There was a problem hiding this comment.
not really. tabScreenFragments do hold fragments for all rendered screens, possibly many, while there should be only a single TabScreenFragment of ours attached to the fragment manager. And this is what these assertions are for.
3 participants
Description
Fixes
updateSelectedTabremoving unowned fragments (fragments that were created by other libraries)Changes
TabScreenFragmentTest plan
This change was tested using the new Expo template in the preview version of Expo Go.
Before, the
react-native-screensremoved the fragment created byexpo-dev-menu. With this fix, everything seems to work.