fix(iOS): header left and right layout on fabric #2248
Conversation
alduzy
changed the title
…-left-and-right-layout-fix
There was a problem hiding this comment.
Looks promising, I just left a question on "whys".
If you're to spend too much time investigating then I think it is not worth it & feel free to say so.
One general note: add link to this PR in comment above the line you added, because it is definitely not obvious why the line should be there.
| @@ -72,6 +72,7 @@ - (void)updateLayoutMetrics:(const react::LayoutMetrics &)layoutMetrics | |||
| self); | |||
| } else { | |||
| self.bounds = CGRect{CGPointZero, frame.size}; | |||
| [self.superview layoutIfNeeded]; | |||
There was a problem hiding this comment.
Okay, so we now trigger layout in the native superview (this is one of system views, beyond control of react) every time we receive a new frame from react.
The solution itself is ok & if it helps I'm for it, but do we have understanding why does it not work without this change? Especially that we do not have such code on Paper & I believe it works fine there?
There was a problem hiding this comment.
Yeah, it does work fine on paper. It looks like the bounds of the subview are set properly but then the frame is given the old values unless we trigger a re-layout of the superview.
There was a problem hiding this comment.
Did you check what happens if you disable view recycling on header subviews in this PR? Im speaking with context of this issue on which we debated earlier today: #2232
There was a problem hiding this comment.
This might be the same issue of the views being swapped due to recycling pool working in stack manner.
There was a problem hiding this comment.
Turning off recycling does not fix this issue
## Description This PR fixes the incorrect position of the custom header items when updating more than one option at the same time. The proposed solution let us remove the previous fix for a similar problem: #2248 which only fixed the issue on fabric. Fixes #432, #2231. ## Changes - forced re-layout of the navigation controller when subviews are updated - removed previous fix - updated `Test432` repro <!-- ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ### After --> ## Test code and steps to reproduce - use `Test432` and `Test2231` to test this fix on both architectures. ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
Description
This PR fixes the header layout mismatch when updating both
headerLeftandheaderRightoptions simultaneously usingnavigation.setOptions.Fixes #2231 .
Changes
Test2231.tsxreproScreenshots / GIFs
Before
After
Test code and steps to reproduce
Test2231.tsxreproChecklist