Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: change update logic #600

Merged
merged 2 commits into from
Aug 18, 2020
Merged

fix: change update logic #600

merged 2 commits into from
Aug 18, 2020

Conversation

WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Aug 12, 2020

Suggested by @kmagiera change that should resolve #432.
We wait with adding to the parent controller until the stack is mounted and has its initial layout done. If we add it before layout, some of the items (specifically items from the navigation bar), won't be able to position properly. Also, the position and size of such items, even if it happens to change, won't be properly updated (this is perhaps some internal issue of UIKit). If we add it when the window is not attached, some of the view transitions will be blocked (i.e. modal transitions) and the internal view controller's state will get out of sync with what's on-screen without us knowing.

@WoLewicki WoLewicki marked this pull request as ready for review August 17, 2020 14:16
@WoLewicki WoLewicki merged commit 858c01e into master Aug 18, 2020
@WoLewicki WoLewicki deleted the @wolewicki/change-update-logic branch August 18, 2020 09:32
@FrankFundel
Copy link

When using this, the header is completely missing on first load. After I switch tabs the header appears.

@WoLewicki
Copy link
Member Author

@FrankFundel we are aware that this solution is not the best one and are working on finding a better one. Meanwhile, can you share a repo with the bug you mentioned? It would help us find all the existing issues.

@WoLewicki WoLewicki mentioned this pull request Aug 25, 2020
WoLewicki added a commit that referenced this pull request Sep 15, 2020
Followup of #600 changing the return condition.
alduzy added a commit that referenced this pull request Jul 9, 2024
## Description

This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- #600
- #613
- #643


The removed `_hasLayout` was initially added by
#600 in
order to resolve an issue:
#432.
However the logic was later changed by
#613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](#432 (comment)).

Fixes #1645.

## Changes

- removed `_hasLayout` variable
- added repros

## Screenshots / GIFs

### Before


https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

### After


https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

## Test code and steps to reproduce

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
alduzy added a commit that referenced this pull request Jul 9, 2024
This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- #600
- #613
- #643

The removed `_hasLayout` was initially added by
#600 in
order to resolve an issue:
#432.
However the logic was later changed by
#613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](#432 (comment)).

Fixes #1645.

- removed `_hasLayout` variable
- added repros

https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…-mansion#2188)

## Description

This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- software-mansion#600
- software-mansion#613
- software-mansion#643


The removed `_hasLayout` was initially added by
software-mansion#600 in
order to resolve an issue:
software-mansion#432.
However the logic was later changed by
software-mansion#613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](software-mansion#432 (comment)).

Fixes software-mansion#1645.

## Changes

- removed `_hasLayout` variable
- added repros

## Screenshots / GIFs

### Before


https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

### After


https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

## Test code and steps to reproduce

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

headerRight randomly position the component incorrectly
3 participants