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.
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
Create separate transition pages to sign the user in / out #8855
Create separate transition pages to sign the user in / out #8855
Changes from 69 commits
0275158
ae4f499
e2daffb
ff3dbaf
37dfb3b
f5ac630
76035f9
1ae8690
732517d
c4a6a67
908a938
4c65e1e
d1800a5
9ac316a
e647868
019dba9
56a79d2
50082e7
5f621ea
7a243c2
328f385
f4b40d4
8ccf60f
60635af
96e7fd4
d47dce8
6dd4573
b490551
174e012
607e48d
dd54853
468c5a0
f901904
853740e
d34921a
f976228
56f9ba8
511cc68
3a158f3
35a18a0
6607d59
ecdbe41
bb8bf06
7e3030c
f291838
326b3b1
ed16b92
a7cb78e
f0bc0f8
3cd291d
f3b265d
a34c511
4729aca
35b5777
320ec95
fe274ed
0921b6c
09e3bf1
b5da43a
1c211f1
5d928e6
2097b8e
e85a80c
dc6fa72
eee230f
dcc3e79
4baed9e
552e13c
0e20ad0
0222143
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor comment that doesn't have to be a blocker .... we only have this code because of a known bug in react-navigation. If possible, we should create a minimal reproduction and a fresh issue in the react-navigation repo, and see if we can either submit a PR to fix it or hire the maintainer to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it would help us though as the navigation becomes available and unavailable when conditionally rendering the navigators e.g. in Neil's case we are opening the app while logged in then logging out then logging in and trying to navigate. Unless the
onReady()
callback can be called multiple times while changing the root navigator. Not really sure what the expected behavior is there, but seems like the maintainer implied it was not a bug and suggested we stop conditionally rendering the navigators (easier said than done of course 😄 ).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we conditionally render the Public and Auth screens rather than the navigators? They both use a stack navigator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into this a while back, while working on this exact same flow. It was ... problematic. I don't remember everything about it, but here's some context including an expo snack. Not sayin' it's impossible, only that I gave up. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think it will be a big change, but if it solves all of our "navigator is not ready" issues then it could be worth looking into... eventually!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent comments here.