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

Fix mobile product name being misaligned in new type scale #4855

Merged
merged 2 commits into from
Mar 15, 2024

Conversation

querkmachine
Copy link
Member

Intended to be a 'quick fix' of the most visible issue mentioned in #4841. There are still potential alignment issues documented below.

Changes

  • Modifies the margin-top of the product name on the mobile breakpoint, if the new type scale is being used.

Screenshots

(All screenshots are in Chromium.)

Before After
Narrow mobile before-narrow-mobile after-narrow-mobile
Mobile before-mobile after-mobile
Tablet+ before-tablet after-tablet

This change principally aims to fix the middle row here ("Mobile") although it does have repercussions on the narrow mobile view too, as the product name appears slightly higher than previously, which may be undesirable.

(Note that the logo and product name always link to the same place, so proximity to one another shouldn't be a problem regarding accessibility.)

Thoughts

In typically frustrating fashion, each browser and screen has a slightly different interpretation of how to align the product name with respect to the logo depending on our dreaded foe, subpixel rounding.

The baseline misalignment between GOV.UK logo and product name on my screens appears to be:

Browser MBP display
(3024×1964, 254 ppi)
External display
(2560×1440, 123 ppi)
Chromium 0 -1px
Firefox (w/ -0.5px hack) -1px 0
Firefox (w/o -0.5px hack) 0 0
Safari +2px -1px

I'm not sure how much pixel nudging we're up to doing to try and minimise the difference between browsers. Experience would tell me that getting it dead-on everywhere is liable to become a caper. That's why this PR only really aims to fix the immediate "this is just way off" alignment issue, rather than fix it in all circumstances.

Todo

Add changelog.

@querkmachine querkmachine requested a review from a team March 12, 2024 16:11
@querkmachine querkmachine self-assigned this Mar 12, 2024
Copy link

github-actions bot commented Mar 12, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 112.86 KiB
dist/govuk-frontend-development.min.js 39.44 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 81.25 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 76.31 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 112.85 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 39.42 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 72.23 KiB 37.61 KiB
accordion.mjs 22.71 KiB 12.85 KiB
button.mjs 5.98 KiB 2.69 KiB
character-count.mjs 22.4 KiB 9.92 KiB
checkboxes.mjs 5.83 KiB 2.83 KiB
error-summary.mjs 7.89 KiB 3.46 KiB
exit-this-page.mjs 17.1 KiB 9.26 KiB
header.mjs 4.46 KiB 2.6 KiB
notification-banner.mjs 6.26 KiB 2.62 KiB
radios.mjs 4.83 KiB 2.38 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.13 KiB 6.11 KiB

View stats and visualisations on the review app


Action run for a3326cf

Copy link

github-actions bot commented Mar 12, 2024

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/components/header/_index.scss b/packages/govuk-frontend/dist/govuk/components/header/_index.scss
index de025ff07..858595250 100644
--- a/packages/govuk-frontend/dist/govuk/components/header/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/header/_index.scss
@@ -64,7 +64,7 @@
   }
 
   .govuk-header__product-name {
-    $product-name-offset: 10px;
+    $product-name-offset: if($govuk-new-typography-scale, 7px, 10px);
     $product-name-offset-tablet: 5px;
 
     @include govuk-font-size($size: 24, $line-height: 1);

Action run for a3326cf

@querkmachine querkmachine marked this pull request as ready for review March 12, 2024 16:45
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4855 March 12, 2024 16:46 Inactive
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

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

Great stuff. I had a go at trying to make this dynamic but it seems non-trivial enough to not be worth it right now. There probably is a way using the height of the text and the header but because this isn't at the baseline of the SVG itself but a little above to align with the text, it's a pain. This is probably the least complex solution.

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.

3 participants