#1061 - have menu open by default no matter if you're logged in or not #1065
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1065 +/- ##
===========================================
- Coverage 97.94% 97.93% -0.01%
===========================================
Files 42 42
Lines 1555 1551 -4
Branches 421 418 -3
===========================================
- Hits 1523 1519 -4
Misses 31 31
Partials 1 1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Generally happy with this but I was thinking about more scenarios about when the user might want the nav menu open, since the issue wants to address moments when the user might feel a bit lost or unsure about where to navigate to. So here's a few questions:
- On successful login, the nav menu remains in the same state after redirection. Do we want it to always open instead? This could aid in the times the user is redirected back to the home page and may not know where to go from there but could interfere with the user's explicit choice to close the nav menu and keep it closed.
- Similarly on logout, user is redirected to homepage and the nav menu remains in the same state. Do we want it to always open in this case? In this instance, the user is effectively being returned to the same state that they arrived at the site, i.e. not logged in and viewing the homepage. Therefore, do we want the menu open like before?
- Finally, a small possible addition. When the user arrives at a 404 page, the nav menu successfully opens (expected behaviour) but we aren't explicitly checking for this in the 404 page e2e test
'should load 404 page correctly'. Should we add a line to check for it here? Or is this covered by the following test,'page refresh should open the navigation drawer'?
|
On a small unrelated note, another thing I just noticed - redirection back to a protected route after successful login works fine for plugins but doesn't for SciGateway routes. For example, let's say the user is on the help page and not logged in. They successfully sign in but are redirected to the homepage instead of back to the help page. Do we want this to happen? If not, we can make a separate issue perhaps. |
|
I think we don't want to overcomplicate things with the menu. Currently, all we do is have the menu open when the site loads. If a user then shuts it, they presumably know how to open and close the menu at that point so I don't think we should auto open it after login etc. I don't think we need to check it on the 404 page test as it's not really a test of the 404 page - the menu is now open in all circumstances after the site loads so that includes the 404 page. As for your latter point, the user isn't redirected from the help page to the login page. The login redirect is to solve the use case of "user clicks on a link, user is unauthenticated so goes to login page, once logged in user should be back at the page the originally clicked on", whereas if a user logs in after visiting the help it's because they've explicitly navigated to the login page via the button. We can redirect in that case as well but it's less clear cut - for example a lot of apps would just direct you to their homepage at that point but some would redirect you back to the page you were previously viewing. |
Description
Basically I just removed the check for logged in users (and also for the login page as I figured we can just have the menu open everywhere by default for simplicity). Also stopped the drawer from closing on token invalidation.
Testing instructions
Agile board tracking
Closes #1061