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

Update layout header component #1902

Merged
merged 2 commits into from
Feb 5, 2021
Merged

Conversation

danacotoran
Copy link
Contributor

@danacotoran danacotoran commented Feb 2, 2021

What

Add data attribute support on the layout header navigation links.

The reason for this is because we are intending to use the layout_header component for the transition checker nav whose links are cross-domain links and need data attributes for tracking.

Why

During development the markup for the account nav (appears on the transition page, and transition checker) was copied over from the layout header component instead of using the component itself.

Screenshot 2021-02-02 at 18 30 08

The component audits are now complaining that we are including the assets for layout_header in static, when we are not using the component.

To fix this problem and reduce repetition, we are removing the hard coded account navigation from static, and using the layout header component in its place: alphagov/static#2408

Visual Changes

https://trello.com/c/9Kjy1hjb

@bevanloon bevanloon temporarily deployed to govuk-publis-layout-hea-vdjl1n February 2, 2021 15:11 Inactive
@danacotoran danacotoran force-pushed the layout_header_adjustments branch from 567460e to bc09f96 Compare February 2, 2021 18:22
@bevanloon bevanloon temporarily deployed to govuk-publis-layout-hea-vdjl1n February 2, 2021 18:22 Inactive
@danacotoran danacotoran marked this pull request as ready for review February 3, 2021 09:41
@bevanloon bevanloon temporarily deployed to govuk-publis-layout-hea-vdjl1n February 3, 2021 09:51 Inactive
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Looks good, only some minor comments.

@danacotoran danacotoran force-pushed the layout_header_adjustments branch from 7c44411 to e44b671 Compare February 5, 2021 15:15
@bevanloon bevanloon temporarily deployed to govuk-publis-layout-hea-vdjl1n February 5, 2021 15:15 Inactive
Add data attribute support on the layout header navigation links. The
reason for this is because we are intending to use the layout header
component for the transition checker nav whose links have data
attributes for tracking.
Another use for data attributes is for the Slimmer logic that
hides/shows different links depending on the user's logged in status.
@danacotoran danacotoran force-pushed the layout_header_adjustments branch from e44b671 to a19a279 Compare February 5, 2021 15:24
@bevanloon bevanloon temporarily deployed to govuk-publis-layout-hea-vdjl1n February 5, 2021 15:25 Inactive
@danacotoran danacotoran merged commit 13f0635 into master Feb 5, 2021
@danacotoran danacotoran deleted the layout_header_adjustments branch February 5, 2021 15:30
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