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

Implement transitional crown in the Header component (v5.0) #4449

Merged
merged 6 commits into from Nov 23, 2023

Conversation

querkmachine
Copy link
Member

@querkmachine querkmachine commented Nov 9, 2023

Implements transitional crown in Frontend 5.0. That is, an asset that utilises the new lockup agreed for Frontend v5 (#1739), but using the St. Edward's crown. Part of #4440.

Changes

  • Adds the new GOV.UK logo lockup (with St. Edward's crown) to the Header component.
  • Applies several changes to how the SVG is included to ensure it's visible to assistive technologies. This is needed as the 'GOV.UK' text is no longer present on the page as text.
    • This uses a combination of role="img" and an embedded <title> element. This combination appears to work consistently across screen readers in Deque's assessment of SVG implementations ("SVG pattern 5") and does not require the use of globally unique ids.
    • The inline fill="currentcolor" definition has been removed as the same style is being applied by CSS.
  • Several HTML structure changes:
    • The .govuk-header__logotype span that wrapped both logo elements has been removed.
    • The .govuk-header__logotype-crown class has been renamed .govuk-header__logotype.
    • The .govuk-header__logotype-text span has been removed.
  • Moves some of the styles applied to the crown icon to apply to the entire logotype.
  • Updates tests for the new SVG.
    • Modified the tests we run on the page's <title> element so that they don't conflict with the SVG's <title> element.

Copy link

github-actions bot commented Nov 9, 2023

📋 Stats

File sizes

File Size
dist/govuk-frontend-5.0.0-beta.1.min.css 114 KiB
dist/govuk-frontend-5.0.0-beta.1.min.js 37.93 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 77.58 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 72.89 KiB
packages/govuk-frontend/dist/govuk/all.mjs 3.86 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 113.64 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 38.11 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.38 KiB

Modules

File Size
all.mjs 69.22 KiB
components/accordion/accordion.mjs 21.47 KiB
components/button/button.mjs 4.5 KiB
components/character-count/character-count.mjs 21.05 KiB
components/checkboxes/checkboxes.mjs 5.63 KiB
components/error-summary/error-summary.mjs 5.89 KiB
components/exit-this-page/exit-this-page.mjs 15.89 KiB
components/header/header.mjs 3.71 KiB
components/notification-banner/notification-banner.mjs 4.34 KiB
components/radios/radios.mjs 4.63 KiB
components/skip-link/skip-link.mjs 3.61 KiB
components/tabs/tabs.mjs 9.47 KiB

View stats and visualisations on the review app


Action run for e346a4a

@querkmachine querkmachine changed the title Implement transitional crown favicons (v5.0) Implement transitional crown in the Header component (v5.0) Nov 9, 2023
@colinrotherham
Copy link
Contributor

Thanks @querkmachine

Would you mind keeping the commit 058b443 separate for history sake? Extra information in the commit message

Did you want to slightly smaller (file size) logo from commit 8e0ce07 too?

@querkmachine
Copy link
Member Author

Would you mind keeping the commit 058b443 separate for history sake? Extra information in the commit message

Done!

Did you want to slightly smaller (file size) logo from commit 8e0ce07 too?

Not done! Wrong crown for this PR. 😋

@colinrotherham
Copy link
Contributor

Not done! Wrong crown for this PR. 😋

😎

@querkmachine querkmachine marked this pull request as ready for review November 20, 2023 15:51
@querkmachine querkmachine requested a review from a team November 20, 2023 15:51
Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this is looking good @querkmachine

Added a few minor comments then will want a CHANGELOG entry and we're spot on

Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work with all this @querkmachine

Would you mind giving it a rebase with main? This PR and #4445 will need merging in order as both have ### Breaking changes but think we're happy to get them in the next v5 beta

@colinrotherham
Copy link
Contributor

@claireashworth is keeping a Google Doc up to date with a re-ordered CHANGELOG so I'll flag this change too

querkmachine and others added 3 commits November 23, 2023 16:27
- Adds logo lockup of St Edward's crown with integrated GOV.UK text
- Removes HTML and classes that used to wrap the logo group as these are no longer needed
- Adjust the vertical offset of the logo to align with productName text
- Tweak attributes to ensure logo is available to assistive technologies
Some minor defects snuck into the previous lockup that was used, potentially introduced due to rounding during compression. These
were not visible at typical sizes, but may have affected how browsers rounded subpixel values.

- There was a 'notch' inside of the G caused by two closely located points being rounded in opposite directions.
- The . character was slightly deformed and not a clean circle.
- Snap points to whole pixel values where very close (e.g. 27.005px to 27px)
- Fix positioning relative to product name

Co-authored-by: Colin Rotherham <work@colinr.com>
colinrotherham and others added 3 commits November 23, 2023 16:27
The link was given `display: inline` at tablet sizes but the product name doesn’t wrap until desktop: #2549
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4449 November 23, 2023 16:27 Inactive
@querkmachine querkmachine merged commit 7a1913e into main Nov 23, 2023
43 of 44 checks passed
@querkmachine querkmachine deleted the transition-crown-v5 branch November 23, 2023 16:39
@romaricpascal romaricpascal mentioned this pull request Dec 8, 2023
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.

None yet

4 participants