Dynamic user splash page #71
Conversation
bc45f6a to
24c7437
Compare
|
👍 I'm going to close #69 and review this PR instead, which is now up on https://nextstrain-dev.herokuapp.com |
There was a problem hiding this comment.
Thanks @joverlee521 and @kairstenfay -- this looks really good. See the line comments for things which need changing.
(i) When logged in /whoami redirects to /users/<userName> but /users doesn't redirect (in this case /users/<userName> looks identical to users. Is this intentional? Seems like it should redirect but this is a minor point & it may be intentional.
(ii) Your usage of <UserDataWrapper> seems like a good way to inject state into downstream components. Alternative solutions would be to use React's Context functionality to only inject it into components that need it, but I'm happy with your solution.
(iii) The usage of empty.png is nice as one day we'll hopefully have images to display for each group. Webpack inlines this into a small base64 string so there's no real performance penalty I can see.
|
|
||
|
|
||
| const FourOhFour = () => { | ||
| // Workaround so 404 page doesn't flash when pages are redirecting |
There was a problem hiding this comment.
This is nice! It removes the flash for me when going straight to docs pages (with or without redirects) but the (this was a flash of the splash page, not the 404 page)
The 404 page still flashes when I click "login" or "logout". In particular, clicking logout causes 2-3 page flashes, including at least one 404. Can you replicate?
There was a problem hiding this comment.
After logging in and logging out, I no longer see 404 flashes.
There was a problem hiding this comment.
They're still there for me. This issue isn't blocking for this PR (the behavior's not introduced here) so i'm going to make an issue with GIFs so that you can see what i'm on about.
Co-authored-by: Jover Lee <joverlee521@gmail.com>
Redirects from `/whoami` to `/users/<username>` is a user is signed in. Redirects to `/users` if there is no user signed in.
Used a workaround found on github (gatsbyjs/gatsby#5329 (comment)) to prevent the index or 404 page from flashing during page redirects.
`user.groups` provides all groups that users are a part of on AWS, but we only care about groups that match our Nextstrain sources.
Assumes that all group urls follows the format `/group/<group-name>`.
Provides a wrapper that fetches user data from the API endpoint "/whoami". Passes the user state to all children through the prop `user`
Refactor pages to use userDataWrapper. Refactor NavBar to use `user` prop instead of having its own user state.
If a user is signed in, they can see a "groups" section on the splash page that lists all of their groups. Currently the group tiles use an "empty" image with a background color. The background colors rotate through the titleColors listed in theme.js.
736e51c to
390b32a
Compare
|
@jameshadfield I've incorporated your requested changes except for:
|
Minor font size changes
|
Thanks @kairstenfay i've retested and it's working well. This is a nice step forward for the website! Note that because cognito groups are now filtered against those specified in
I never requested those two changes! I only mentioned |
Understood. I simply wanted to raise the two remaining potential changes in a single comment. Additionally, it just occurred to me that we never implemented a redirect from |
This PR builds on top of #69
If a user is signed in, the splash page has a groups section that dynamically creates tiles for the user's groups. These tiles are empty images with a background color from
theme.titleColors.In order to pass the user data to the index page, I created a
UserDataWrapperthat fetches user data from/whoamiand passes the user state to each child element through theuserprop. I'm not entirely sure how to manage a "global" user data store within gatsby when the authentication is handled on the server side.