fix(iOS, Fabric): wrong headerCenter layout after native dismissal #2263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alduzy
approved these changes
Jul 29, 2024
ja1ns
pushed a commit
to WiseOwlTech/react-native-screens
that referenced
this pull request
Oct 9, 2024
…oftware-mansion#2263) ## Description This PR fixes software-mansion#2232 by disabling view recycling in `RNSScreenStackHeaderSubview` view component, as no better solution was found. The following sequence leads to the bug: 1. User clicks back button 2. The header subviews are properly removed and put in recycle pool in the same order they were mounted (recycle pool is a stack). 3. User pushes again just-popped-screen. 4. React Native reuses view from recycle pool, but `headerCenter` is to be mounted first, so RN reuses the view that was previously `headerRight` (because it is on top of the recycle pool stack). Similarly the second view in recycle pool, which was previously `headerCenter` is now repurposed as `headerRight`. 5. RN sets correct frames for both subviews. 6. System sets wrong frame for the new `headerCenter` due to layout triggered by `setNavigationBarHidden` in `updateViewController` of `RNSScreenStackHeaderConfig`. The `setNavigationBarHidden` is called, because there is difference in value of the prop between the screens (see issue for repro). The only fact that matters is the one, that native layout is triggered & it computes the `width` incorrectly <- it looks like UIKit does hold some cache for layout computations & when seeing the recycled view it just bonks it with it's previous frame, for some reason not observing that deeper in view hierarchy contents have changed. I've tried to invalidate layout of the subview in various places that could make sense, e.g. `prepareForRecycle`, just before computing native layout in `updateViewController`, etc. for no result. To avoid putting more debugging hours into this particular issue I went with disabling view recycling for header subviews, originally suggested by @janicduplessis in the issue description. ### Open questions We would want to answer those at some time in the future: 1. [ ] What's the difference between JS-initiated dismissal and native one, in this very case? Where does the difference in behaviour come from? (JS-initiated works properly) 2. [ ] In what other way can we solve this w/o disabling view recycling? Fixes software-mansion#2232 ## Changes Disable view recycling in `RNSScreenStackHeaderSubview` view component. ## Test code and steps to reproduce `Test2232` ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes #2232 by disabling view recycling in
RNSScreenStackHeaderSubviewview component, as no better solution was found.The following sequence leads to the bug:
headerCenteris to be mounted first, so RN reuses the view that was previouslyheaderRight(because it is on top of the recycle pool stack).Similarly the second view in recycle pool, which was previously
headerCenteris now repurposed asheaderRight.headerCenterdue to layout triggered bysetNavigationBarHiddeninupdateViewControllerofRNSScreenStackHeaderConfig. ThesetNavigationBarHiddenis called, because there is difference in value of the prop between the screens (see issue for repro). The only fact that matters is the one, that native layout is triggered & it computes thewidthincorrectly <- it looks like UIKit does hold some cache for layout computations & when seeing the recycled view it just bonks it with it's previous frame, for some reason not observing that deeper in view hierarchy contents have changed.I've tried to invalidate layout of the subview in various places that could make sense, e.g.
prepareForRecycle, just before computing native layout inupdateViewController, etc. for no result.To avoid putting more debugging hours into this particular issue I went with disabling view recycling for header subviews, originally suggested by @janicduplessis in the issue description.
Open questions
We would want to answer those at some time in the future:
Fixes #2232
Changes
Disable view recycling in
RNSScreenStackHeaderSubviewview component.Test code and steps to reproduce
Test2232Checklist