Skip to content

Header updates - breaking changes #1058

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

Open
wants to merge 125 commits into
base: main
Choose a base branch
from
Open

Conversation

anandamaryon1
Copy link
Contributor

@anandamaryon1 anandamaryon1 commented Oct 24, 2024

@anandamaryon1
Copy link
Contributor 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)
@colinrotherham
Copy link
Contributor

We're getting close 😱

Merged with main again

@colinrotherham colinrotherham marked this pull request as ready for review May 13, 2025 10:29
@colinrotherham
Copy link
Contributor

@frankieroberto @paulrobertlloyd Can I check which one of these is correct?

Could do with some fixes adding

Mobile

  • Focus outline encloses button
Screenshot 2025-05-22 at 10 05 50
  • No grey border to left of button
Screenshot 2025-05-22 at 10 06 01

Tablet

  • Focus outline does not enclose button
  • Focus outline appears underneath button (current release is over button)
Screenshot 2025-05-22 at 10 06 16
  • Grey border to left of button
Screenshot 2025-05-22 at 10 06 10

Copy link

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.

5 participants