fix: invalid header background colour #1560
Merged
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
kkafar
force-pushed
the
@kkafar/fix-1391
branch
2 times, most recently
from
6331f3f
to
80e4a27
Compare
kkafar
force-pushed
the
@kkafar/fix-1391
branch
from
80e4a27
to
e8cac66
Compare
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
Fixes #1391
Fixes #1477
The code added in #1228 did not handle case when both view controller and "next view controller" were
nullas they are most of the times (in tested scenario -> Test1391)! This caused view controller to be updated almost every time, which counter-intuitively leaded to invalid header state.It is not clear to me why it was the case, as it would seem to me that updating view controller more frequently would rather lead to more up-to-date header state, however it is not the case.
Note 1: The issue did not appear on iOS 15+ (everything works just fine there)
Note 2: I reproduced & fixed it using simulator with iOS 13.2 (but it still works on more recent iOS too)
Note 3: While debugging, it occurred to me that there are some concurrency issues in this code (
RNSScreenStackHeaderConfig#updateViewControllerIfNeeded). As I was repeatedly opening & closing modal view this method was executed with various configurations ofnonnull&nullvalues for_screenView.controller,vc.parentViewController, etc. This most likely requires further investigation.Note 4: I also ensured that the problem resolved by #1228 was not brought back.
Changes
Added checks for case when both
view controllerandnext view controllerare null (see the code).Test code and steps to reproduce
Test1391in test examples.Checklist