-
Notifications
You must be signed in to change notification settings - Fork 108
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
Header updates - breaking changes #1058
base: main
Are you sure you want to change the base?
Conversation
@paulrobertlloyd found an issue with your logo SVG change (#1047), when printing the NHS logo no longer shows. Not looked into the code but I assume it's just missing a rule to change it's colour from white to blue on print? |
c2ccf58
to
fa05a47
Compare
f5a7da9
to
08add40
Compare
Removes the link to `"/"` labelled `"Home"`, which is currently hardcoded, and only shows up within the navigation menu on mobile screen widths. This link may not be appropriate for all services, which might not have a homepage, or might use a different path for it. It is also unclear whether having a homepage link is always needed in the navigation if the NHS logo also goes to the homepage.
9ab62ce
to
a1ee8c1
Compare
@paulrobertlloyd could we avoid force pushing to this branch for a bit? Makes it harder to maintain the other pull requests into this branch. We can sort out the commit history at the end? |
Fixes issue with org header font size #892 |
Potential new issue to include in the PR: Making the header work without JS enabled and progressively enhancing the responsive layout via JS. Currently the responsive layout without JS is a bit broken, with the nav links overflowing rather than wrapping. Update: |
* Add current item indicator for header navigation Add support for indicating the current section, using either `current: true` (if the user is currently on this exact page) or `active: true` (if the user is in that section but not necessarily that exact page). When displayed in the regular view within the blue header, the active links have a light-grey border at the bottom. When displayed in the expanded menu view (eg on mobile), they have a blue border to their left. * Remove custom class * Fix focus style on expanded menu item * Changed current item indicator in white nav to blue * Bump mobile current item indicator to 8px * Set current item in example header-org-white variant * Update backstop images * Fix * Bump mobile nav current idem indicator to 6px * Remove duplicate entries from changelog merge issues * Add entry to changelog * Update backstop images * Remove excess whitespace * Fix: focus link in white header should always have black border --------- Co-authored-by: Paul Robert Lloyd <[email protected]>
This adds the new current and active boolean options for primaryLinks in the header added in #1067 to the documentation in the component README.
* Update header navigation label --------- Co-authored-by: Frankie Roberto <[email protected]> Co-authored-by: anandamaryon1 <[email protected]>
* Add account links to header * Don’t link to header example with custom HTML * Update backstop references for header component after adding account information to this component * Remove unused backstop images for header component * Add backstop images for header component with account links
Accessibility and device testingTested with NVDA, JAWS and VoiceOver, all work as expected. Tested on Safari on Mac, and Edge and Chrome Windows. Android devices Galaxy 21 Samsung browser and Pixel 6 Chrome, all as expected. Tested on iOS Safari, all as expected on version 15 and above. IssuesTwo issues discovered on Safari 14 (iPhone 8, iPad mini 2019 & iPad air 4) and below:
These issues do not affect the current header implementation. Raises a wider question of our browser support which has not been updated in 3 years. nhsuk/nhsuk-service-manual#2063 |
Description
Bringing together a series of breaking changes from the following PRs:
Checklist