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

Adjust crown alignment in the heading component #2134

Merged
merged 1 commit into from
Jul 6, 2021

Conversation

injms
Copy link
Contributor

@injms injms commented Jun 14, 2021

What

Adjust the crown in the header component slightly.

Also swapped out a govuk- class for a gem-c-class.

Why

So it better aligns with the "GOV.UK" logotype. And govuk- is the namespace for the Design System - classes in the gem should be gem-c-.

Visual Changes

Before After Diff
component-guide__layout_header__preview - firefox live component-guide__layout_header__preview - firefox preview component-guide__layout_header__preview - firefox diff

@bevanloon bevanloon temporarily deployed to govuk-publis-adjust-hea-tmyhb3 June 14, 2021 18:19 Inactive
@injms injms force-pushed the adjust-heading-crown-alignment branch from 2bea325 to 0568465 Compare June 14, 2021 18:25
@bevanloon bevanloon temporarily deployed to govuk-publis-adjust-hea-tmyhb3 June 14, 2021 18:25 Inactive
@nnagewad
Copy link

Looks good @injms

@christopherthomasdesign
Copy link
Member

Hi all, sorry to jump in!

I've just been overlaying the header in the Design System over these because they looked a bit different (the Design System one is the 50% opacity one on top).

The crown and GOV.UK text both look like they have more or less the same alignment as the Design System one, but the padding around the logo seems to vary quite a lot across the different headers:

govuk-header-comparison

Maybe I'm missing the intent of the change here – is this what's being fixed?

@injms
Copy link
Contributor Author

injms commented Jun 15, 2021

@christopherthomasdesign - for some reason the crown was looking really misaligned - it was not lining up with "GOV.UK" as expected. I've updated the description with a visual diff that better shows the difference.

There are some bigger updates coming to the header across www.gov.uk soon, so this is more of a quick fix to tide us over until those changes are live.

I don't know why the padding is different compared to the header in the Design System - happy to have a chat after I've dug a bit more into the history of this, but for the moment I think it's outside the scope of this pull request.

@36degrees
Copy link
Contributor

for some reason the crown was looking really misaligned - it was not lining up with "GOV.UK" as expected.

Assuming you're seeing this as part of the public layout rollout, I'm pretty sure this'll be related to you switching from the v1 font to the v2 font as they have different baselines. We tweaked the baseline for the v2 version of the font so that it better aligned with Arial (or most other fonts) so that the vertical alignment is correct if the font fails to load.

There's a number of 'corrections' in GOV.UK Frontend depending on whether compatibility mode is on or off. Fundamentally though, the crown and the logotype should line up without you needing to apply additional corrections.

Are there any other overrides being applied to any other elements? I think there must be a 'correction' somewhere that's tweaking the alignment for the v1 font that needs to be removed, rather than adding yet another override.

Happy to chat about this if that's useful.

Copy link
Contributor

@chao-xian chao-xian left a comment

Choose a reason for hiding this comment

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

Thanks @36degrees @christopherthomasdesign but this is an issue we need to fix now as a priority and can investigate your comments later. Hope that's ok.

The crown was misaligned - it was lower than it should be in relation to
the text "GOV.UK". This corrects that alignment.

Also swaps out a `govuk-` class for a `gem-c-`class.
@injms injms force-pushed the adjust-heading-crown-alignment branch from 0568465 to f40a78c Compare July 6, 2021 11:05
@injms injms merged commit a5fa854 into master Jul 6, 2021
@injms injms deleted the adjust-heading-crown-alignment branch July 6, 2021 11:11
@injms
Copy link
Contributor Author

injms commented Jul 6, 2021

I've raised this hack as an issue so it doesn't get lost and we can investigate further when time permits - #2187

@chao-xian
Copy link
Contributor

To be clarify, the PR fixes this issue:
Screenshot 2021-07-06 at 11 40 46

injms added a commit that referenced this pull request Jul 6, 2021
Includes:
 - Adjust crown alignment in the heading component (#2134)
@injms injms mentioned this pull request Jul 6, 2021
injms added a commit that referenced this pull request Jul 12, 2021
Previous fix in #2134 was a hack - this fixes the alignment properly by
updating the dimensions of the crown to match the crown dimensions in
the Design System's header component.

Many thanks to @kr8n3r for spotting the reason why this misalignment was
happening.

Fixes #2187.
MartinJJones pushed a commit that referenced this pull request Jun 4, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jun 14, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jun 19, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jun 24, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 3, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 5, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 9, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 9, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 11, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 12, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
MartinJJones pushed a commit that referenced this pull request Jul 15, 2024
In govuk-frontend v5, the crown SVG has been updated to also include the "GOV.UK" text.

- Updated the crown logo SVG in required components
- Removed the `.gem-c-header__logotype` class in `_layout-header.scss` as it is not longer required. This fixed an alignment issue with the previous implementation of the logo - #2134
-Created a new partial so that a variant of the crown logo SVG without "GOV.UK" can be used on the homepage
- Remove references to the crown fallback. In govuk-frontend v5.0.0, the crown fallback image for IE8 has been removed so we need to remove all references to this asset in the gem.
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.

6 participants