Skip to content

Conversation

@arthurdenner
Copy link
Contributor

@arthurdenner arthurdenner commented Jun 7, 2018

What kind of change does this PR introduce?
Bug fix. Closes #852.

What is the current behavior?
After signing out, the UI breaks because the UserMenu component is connected to the store and reacts to the user property being updated. (See #852)

What is the new behavior?
Instead of being connected to the store, the UserMenu component receives the user and userMenuOpen props from the Navigation component, which also decides if the UserMenu should render or not based on the user property.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

@CompuIves
Copy link
Member

This is really nice @arthurdenner! Thank you so much for building this 😄. Merging it in now!

@CompuIves CompuIves merged commit d9402f6 into codesandbox:master Jun 8, 2018
@arthurdenner arthurdenner deleted the bugfix/fixing-ui-updates-after-signout branch June 8, 2018 13:26
CompuIves added a commit that referenced this pull request Jun 8, 2018
@arthurdenner
Copy link
Contributor Author

@CompuIves, sorry for bothering you, but I saw that you reverted this PR and now the sign out is broken again. Also, is there any way to sign in with GitHub during development besides copying the jwt token?

@CompuIves
Copy link
Member

Hey! We have to look at this one again, when deploying this one people weren't able to look at their sandbox when signed in (related #894, #893). I completely forgot that UserMenu is used in two places: in the Editor and the other pages. They're a bit different, we need to change the one in Editor as well to make it work.

@arthurdenner
Copy link
Contributor Author

Sorry, I didn't notice this at all.

@arthurdenner
Copy link
Contributor Author

@CompuIves, I looked at this problem again and noticed that if we use the isLoggedIn property instead of the user property to compute what to render in the Navigation component, the UI doesn't break (and nothing else needs to be changed).

Can you confirm this?

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.

After I got out of my profile, then I need to refresh the page to return to the main menu

2 participants