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

Header updates - breaking changes #1058

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

Conversation

@anandamaryon1
Copy link
Collaborator Author

@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?

@frankieroberto
Copy link
Contributor

@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?

)

Change the primaryLinks in the Header component to use `href` and `text` param names instead of `url` and `label`.

This improves consistency with other components.

The previous param names are still supported for backwards-compatibility (but could be dropped in future)
@anandamaryon1
Copy link
Collaborator Author

Fixes issue with org header font size #892

@anandamaryon1
Copy link
Collaborator Author

anandamaryon1 commented Dec 18, 2024

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:
Created an issue: #1093

frankieroberto and others added 9 commits December 18, 2024 17:46
* 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
@anandamaryon1
Copy link
Collaborator Author

Accessibility and device testing

Tested 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.

Issues

Two issues discovered on Safari 14 (iPhone 8, iPad mini 2019 & iPad air 4) and below:

  1. Flexbox gap is not supported.
  2. Search input gets styling applied to round it's corners.

These issues do not affect the current header implementation.

image
image
image

Raises a wider question of our browser support which has not been updated in 3 years. nhsuk/nhsuk-service-manual#2063

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants