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

Replace bespoke nav with contents list component #1877

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Conversation

maxgds
Copy link
Contributor

@maxgds maxgds commented Oct 14, 2020

This removes the bespoke 2 column layout for parts navigation and replaces it with the contents-list component. This brings in a header of "Contents" and makes the navigation more consistent with the rest of the site which is good for accessibility.
https://trello.com/c/LbVmpaK6/441-inconsistent-naming-of-table-of-contents
Sample page: https://www.gov.uk/foreign-travel-advice/switzerland

Before

Screenshot 2020-10-14 at 15 03 41

After

Screenshot 2020-10-14 at 15 03 31

@bevanloon bevanloon temporarily deployed to government-f-replace-na-mphh9u October 14, 2020 14:05 Inactive
@bevanloon bevanloon temporarily deployed to government-f-replace-na-mphh9u October 14, 2020 14:10 Inactive
@maxgds maxgds marked this pull request as ready for review October 14, 2020 14:26
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

@owenatgov
Copy link
Contributor

Non-blocking comment that I noticed post approval: The contents section is now quite snug to the left side on mobile.
Screenshot 2020-10-15 at 16 40 11

@bevanloon bevanloon temporarily deployed to government-f-replace-na-mphh9u October 19, 2020 08:23 Inactive
@maxgds
Copy link
Contributor Author

maxgds commented Oct 19, 2020

Non-blocking comment that I noticed post approval: The contents section is now quite snug to the left side on mobile.

Thanks, I realised it was too far over at all widths, actually. Have amended, could you re-check, please?

Copy link
Contributor

@injms injms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🚢

@maxgds maxgds merged commit 54f1a79 into master Oct 19, 2020
@maxgds maxgds deleted the replace-nav branch October 19, 2020 10:39
kevindew added a commit that referenced this pull request Aug 6, 2021
There is a bug where these navigation links are sending two GA events
per click. This is occurring because the contents-list component has an
undocumented [1] feature of tracking links which was presumably not
known about when this code was changed to use this component [2].

The resolution for this is to remove the usage of track links on the nav
element and just use the components one.

[1]: https://github.com/alphagov/govuk_publishing_components/blob/147a825fc7608eb3dfdaab0d418c5b2fdaaf08aa/app/views/govuk_publishing_components/components/docs/contents_list.yml
[2]: #1877
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.

4 participants