-
Notifications
You must be signed in to change notification settings - Fork 23
[DO NOT MERGE] Add fix for menu height when zoomed #5021
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
Draft
andysellick
wants to merge
13
commits into
main
Choose a base branch
from
fix-focus-order-super-nav-header--patch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Restructure the HTML used in the super navigation menu by: - moving the dropdown menu HTML directly after the nav button used to control the open/close state - making the govuk logo (links to the homepage) a child of the navigation element - updating container classes to work with the new structure This ensures the super navigation menu is operable in NVDA and JAWS, when the navigation "menu" is open and the arrow keys are used to navigate, it will now move to the links in the dropdown menu instead of first moving to the search button
- Removed the nav menu button container styles as they are no longer wrapped in a div used as a "button container". - Removed styles to float the buttons to the left of the button container, this approach is no longer required with the use of Flexbox
Adds the transparent bottom border back in and only display it when forced-colors mode is active.
Flexbox is used to ensure the super nav menu still renders as expected following the restructure of the HTML, this works by: - Ensuring the govuk logo takes up all of the available space in the super nav menu, "pushing" the navigation links all the way to the right - Ensuring the govuk logo does not shrink at smaller screen sizes - For the dropdown menu, the order is set to 1 to ensure it always appears after the super navigation buttons which have a default order of 0. It is then given a width of 100% to ensure it always wraps onto a new line - Removed the until: 300px media query that set padding-top to 80px, this was previously included to ensure the dropdown menu contents did not sit next to the navigation buttons. This approach is no longer required with the use of flexbox
Ensure the search button remains in the same position following the change to the HTML structure and use of flexbox.
Remove the right margin from the govuk-logo to ensure the super nav menu buttons do not wrap onto a new line when the screen width is 360px ($chevron-breakpoint) govuk-frontend includes spacing to the right of the logo to offset it from the product name as we do not include a product name in the super navigation menu, we can set the right margin to 0
Ensure the dropdown menu background is offset from the menu height and takes up the full screen width The bottom border was removed from the dropdown and now applied using a box-shadow to ensure it is only displayed when the dropdown menu is dispalyed
This ensures the nav links don't blend into the grey background for users that increase their text scale
The GOV.UK logo is now included as a child of the nav element, but this still needs to be included when printing. This change groups together the navigation elements we want to remove when printing in the CSS, instead of adding the `govuk-!-display-none-print` utility class in several locations throughout the view template.
IE10 does not support flexbox, when `display: block` is used the navigation links are moved outside of the navigation header. Switching to `display: inline-block` ensures the nav links are still accessible in browsers that do not support flexbox. Removed `float: left` as it is not needed when testing in IE10
Updated the html fixture to reflect the new HTML structure used in the layout-super-navigation-header component
Removed the `transparent-bottom-border` class and moved the styles to the `gem-c-layout-super-navigation-header__container` The bottom border is now only applied in forced colours mode, having `transparent-bottom-border` in the HTML could imply that it is set in all cases
c687774
to
bca8bc9
Compare
Nice, thanks for this @andysellick 👍 I've created a new card to further improve the super navigation menu component and added it to our backlog, I've included a link to this PR as well so we can look at making the change and complete browser testing before implementing |
bca8bc9
to
af2bf8c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What / why
Adds a fix for the problem of making the background of the dropdown menus (light blue) full width.
Note
100000px
is a deliberately huge value to ensure the elements always reach the edge of the page. Might be safer to make them even wider?cc @MartinJJones
Visual Changes
None.