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

Alter use of pseudo-underline mixin to allow for different button sizes #2501

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

owenatgov
Copy link
Contributor

What

Adds options to the pseudo-underline mixin within the super nav header to allow them to take different left and right attributes based on the button using the mixin.

Why

Led by a bug report from Stephen McCarthy that the underline on the mobile menu button looks a bit weird.

The reason I've made the choice to alter how the mixin is initiated rather than just change the spacing is that this initial spacing is a conscious choice made in #2483 This maintains the intent of 2483 whilst fixing this issue.

Card

Visual Changes (mobile only)

Before

Screenshot 2021-12-08 at 14 14 13

After

Screenshot 2021-12-08 at 14 14 25

@owenatgov owenatgov force-pushed the super-nav-menu-button-underline branch from 353be35 to 244b22b Compare December 8, 2021 14:23
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2501 December 8, 2021 14:23 Inactive
@owenatgov owenatgov marked this pull request as ready for review December 8, 2021 14:23
@mia-allers-gds
Copy link

Looks much better, thank you!

Copy link
Contributor

@chao-xian chao-xian left a comment

Choose a reason for hiding this comment

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

Nice work Owen!

@owenatgov owenatgov merged commit c8ae3ea into master Dec 8, 2021
@owenatgov owenatgov deleted the super-nav-menu-button-underline branch December 8, 2021 14:35
@danacotoran danacotoran mentioned this pull request Dec 9, 2021
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