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

Make mobile menu toggle a button #2283

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

andysellick
Copy link
Contributor

  • was formerly a link, which failed accessibility requirements because it behaved like a button
  • uses existing JS for this (header-navigation.js, in govuk_publishing_components for reference, fact fans)
  • copied and modified the styling from govuk-template (as it no longer applies) in order to preserve the look of the element exactly, CSS may therefore not be as nice as it could be, but this should all be replaced by the proper header at some point anyway

Screenshot 2020-09-24 at 11 11 26

Should be no visual changes.

Trello card: https://trello.com/c/2bL11sIv/414-mobile-menu-button-looks-like-a-link


Interesting note - this code for the menu toggle is in static (obviously). The JS that controls it (and also the search toggle for mobile, just above it) is in govuk_publishing_components. The menu that appears when you toggle this button appears to come from whitehall (https://github.com/alphagov/whitehall/blob/master/app/views/layouts/frontend.html.erb).

The sooner we get the header consolidated into one component the better.

- was formerly a link, which failed accessibility requirements because it behaved like a button
- uses existing JS for this (header-navigation.js, in govuk_publishing_components for reference, fact fans)
- copied and modified the styling from govuk-template (as it no longer applies) in order to preserve the look of the element exactly, CSS may therefore not be as nice as it could be, but this should all be replaced by the proper header at some point anyway
@bevanloon bevanloon temporarily deployed to govuk-static-make-mobil-bar7pf September 24, 2020 10:15 Inactive
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.

Checked with @selfthinker about whether this needs to look like a button, or whether this needs to be a button element - and she's confirmed that this needs to be a button element. So all 👍

🧪👨🏻‍🎤🙆🏻‍♂️

@andysellick andysellick merged commit b8a30e4 into master Sep 24, 2020
@andysellick andysellick deleted the make-mobile-menu-toggle-a-button branch September 24, 2020 16:14
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