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

Remove left border from the super nav menu button #4631

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

MartinJJones
Copy link
Contributor

@MartinJJones MartinJJones commented Feb 14, 2025

What

Removed the 1px left border style, and the hide_button_left_border option from the super navigation menu component.

Why

  • To align the menu button styling on gov.uk with the styling used on the gov.uk homepage
  • The hide_button_left_border option is no longer as it is now removed by default

Trello card

Visual Changes

The failing visual regression tests in Percy are expected, screenshots of how the changes will appear on GOV.UK are included below.

Mobile

Before After
menu-mobile-before menu-mobile-after

Desktop

Before After
menu-desktop-before menu-desktop-after

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4631 February 14, 2025 11:13 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4631 February 14, 2025 13:28 Inactive
@MartinJJones MartinJJones marked this pull request as ready for review February 14, 2025 13:39
Copy link
Contributor

@jon-kirwan jon-kirwan left a comment

Choose a reason for hiding this comment

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

All looks good to me! I just have one additional suggestion - I think the border-left styles below can be removed since they seem to only appear when JS is disabled.

.gem-c-layout-super-navigation-header__navigation-item-link-inner {
border-left: 1px solid $button-pipe-colour;
padding: govuk-spacing(1) $after-link-padding;
}
.gem-c-layout-super-navigation-header__navigation-item-link-inner--blue-background {
border-left: 0;
border-right: 1px solid $pale-blue-colour;
}
.

Disabling JS does show some other nav layout issues, but I'll log a separate issue for those

- Updated the CSS to remove the left border from the menu button in the super navigation menu, this aligns with the styling used on the gov.uk homepage
- Removed the `hide_button_left_border` option, this is no longer required as it is now the default style, this was used on the gov.uk homepage to remove the left border from the super navigation menu button
@MartinJJones MartinJJones force-pushed the remove-menu-button-left-border branch from 2e25311 to d3af7e0 Compare February 17, 2025 11:52
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-4631 February 17, 2025 11:52 Inactive
@MartinJJones
Copy link
Contributor Author

MartinJJones commented Feb 17, 2025

All looks good to me! I just have one additional suggestion - I think the border-left styles below can be removed since they seem to only appear when JS is disabled.

@jon-kirwan good spot!, I've made the suggested change, thanks

Disabling JS does show some other nav layout issues, but I'll log a separate issue for those

Thanks, just let me know if I can help with anything on this 👍

Copy link
Contributor

@jon-kirwan jon-kirwan left a comment

Choose a reason for hiding this comment

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

Great, changes look good to me. Thanks!

Forgot to approve Percy! Done that now.

@MartinJJones MartinJJones merged commit e26f066 into main Feb 18, 2025
12 checks passed
@MartinJJones MartinJJones deleted the remove-menu-button-left-border branch February 18, 2025 11:51
@MartinJJones MartinJJones mentioned this pull request Feb 20, 2025
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.

3 participants