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

Navigation: Remove selected state from pages that are not the home page #5216

Closed
1 task done
tochwill opened this issue Jan 20, 2020 · 4 comments · Fixed by #5634
Closed
1 task done

Navigation: Remove selected state from pages that are not the home page #5216

tochwill opened this issue Jan 20, 2020 · 4 comments · Fixed by #5634
Assignees
Labels
ux To be reviewed by UX before merging

Comments

@tochwill
Copy link
Contributor

tochwill commented Jan 20, 2020

Is your feature request related to a problem? Please describe.
Currently, the first item in the navigation is highlighted as active no matter which page the user is on. This is incorrect for all pages other than the homepage. After discussions with the UX team and various stakeholders, it has been decided that we can't consistently determine which navigation item a particular page falls under. To remove the confusing scenario where the first item is selected when the user is not on that page, we will only highlight the homepage link in the nav if the user is on that page, otherwise we will not highlight anything.

This is a solution that we will be adopting temporarily until we are able to define a solution to know which navigation item should be highlighted as active.

Describe the solution you'd like
Check to see if the visited URL matches the URL for the homepage in config, and mark that nav item as active if the user is on that page. Otherwise, do not mark any items as active.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Testing notes
Test that the homepage link is marked as active when visiting the homepage, and nothing is marked as active for other pages.

Dev insight: Will Cypress tests be required or are unit tests sufficient? Will there be any potential regression? etc

  • This feature is expected to need manual testing.

Additional context
Add any other context or screenshots about the feature request here.

@tochwill tochwill added Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. ws-home labels Jan 20, 2020
@tochwill tochwill added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jan 22, 2020
@tochwill
Copy link
Contributor Author

Blocked on #4833 (release of the new navigation)

@tochwill tochwill removed the Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. label Jan 22, 2020
@tochwill tochwill added the ux To be reviewed by UX before merging label Jan 23, 2020
@tochwill tochwill added blocked This issue should not be worked on until another internal issue is completed - see desc for details and removed blocked This issue should not be worked on until another internal issue is completed - see desc for details labels Feb 4, 2020
@tochwill
Copy link
Contributor Author

tochwill commented Feb 5, 2020

We had to revert the nav, will unblock when it's live!

@AlistairGempf AlistairGempf removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Feb 21, 2020
@AlistairGempf
Copy link
Contributor

We had to revert the nav, will unblock when it's live!

We're live!

@AlistairGempf
Copy link
Contributor

Following discussion on Slack, we've decided to select any page that is in the config and is the current page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ux To be reviewed by UX before merging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants