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 logic error, unless I'm misunderstanding #3

Closed
wants to merge 1 commit into from

Conversation

geirman
Copy link
Contributor

@geirman geirman commented Mar 29, 2016

would make sense that foundUser: false if COULD NOT FIND USER. I considered also changing the logout to COULD NOT FIND USER IN LOCALSTORAGE. Let me know if you want me to roll that in as well.

@tgoldenberg
Copy link
Collaborator

@geirman Good spot on this one. I think what I was trying to convey was more like fetchedUser: true. At the time I was probably so into the code that I didn't think of making the intention legible to other devs. Maybe we can change the name of the field in state? Just have to make sure it doesn't affect any child components....

@geirman
Copy link
Contributor Author

geirman commented Mar 29, 2016

@tgoldenberg did you fetch user even though? If I'm reading this right, you've attempted to retrieve user from asyncstorage, if you get a user back you do stuff with that user object (parse, evaluate, then either log them in

@geirman geirman closed this Mar 29, 2016
@geirman geirman reopened this Mar 29, 2016
@geirman
Copy link
Contributor Author

geirman commented Mar 29, 2016

@tgoldenberg is the user fetched at that point? Looks to me like you've attempted to get the user object from asyncstorage, but it came back null

@tgoldenberg
Copy link
Collaborator

@geirman so down in the render function, it renders a loading screen if userFound is false.
if (! foundUser) { return this._loading(); }
If user is null, the app renders the landing page, instead of logging them into the app and sending them to the dashboard.
What we can do is rename the field to userFetched instead of userFound to be more clear. But not setting it to true would cause the spinner to render indefinately.

@geirman
Copy link
Contributor Author

geirman commented Mar 29, 2016

Ah, ok... I see. I'll withdraw the PR then. Thanks for explaining.

@geirman geirman closed this Mar 29, 2016
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.

None yet

2 participants