refactor(iOS): update view controller once per transaction when adding header subviews #2623
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/update-view-controller-once-per-transaction
branch
2 times, most recently
from
8109853 to
a64e8c6
Compare
|
Lint started failing w/o any changes to the PR and for no apparent reason (before force push it passed). Moreover it has started failing on every single PR 🤷🏻♂️ indicating separate issue. |
kkafar
force-pushed
the
@kkafar/update-view-controller-once-per-transaction
branch
from
a64e8c6 to
45f186e
Compare
kkafar
deleted the
@kkafar/update-view-controller-once-per-transaction
branch
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
In #2466 I've introduced
RCTMountingTransactionObservingforRNSScreenStackHeaderConfigcomponent.Now we can use this to reduce amount of calls to
updateViewControllerIfNeeded(causing layout passes) when adding subviews.I'm not changing the
unmountpath of the code as we have some more additional logic there which requires some morecareful consideration.
This also aligns us with Paper implementation. Note that it has been only implemented this way, because at the implementation time there was no way
to run this code on transaction completion.
Changes
- [RNSScreenStackHeaderConfig updateViewControllerIfNeeded]is now called only once per transaction when adding subviews to header config.Test code and steps to reproduce
Test432, TestHeaderTitle, Test2552 - see that there are no regressions.
Note
During testing I've found that there is a bug with subview layout when modifying subviews after first render. This is not a regression however. Notice the wrong position of header right on video below 👇🏻 (Test432)
Screen.Recording.2025-01-21.at.13.33.55.mov
Checklist