Fix padding on GOV.UK logo affecting hover and focus states #2171
Conversation
36degrees
requested review from
CharlotteDowns,
christopherthomasdesign,
hannalaakso and
vanitabarrett
36degrees
changed the title
There was a problem hiding this comment.
I really like this change!
The class govuk-header__logo also seems to have some right padding for menu items. Do we know why there was right padding on both the govuk-header__logo and govuk-header__product-name previously, is it for responsive views or another use case?
There was a problem hiding this comment.
This is a nicely documented tidy-up @36degrees 👍
However, it seems that there's now some extra margin to the left of the product name when it's on a new line. The GOV.UK text also seems to have slightly moved. Preview.
When the header includes a product name, the homepage link’s underline (on hover) and the focus state take up an additional 10px on the right hand side, because of the 10px right padding on `govuk-header__product-name`. This padding is there to ensure there is a gap left between the product name and the menu button [1]. Remove it, and add a 10px margin to `govuk-header__link--homepage` instead which achieves the same result but without adding an additional 10px to the hover and focus states. [1]: df413cf
When the header does not include a product name, the homepage link’s underline (on hover) and the focus state take up an additional 5px on the right hand side, because of a 5px margin on `govuk-header__logotype`. This margin has been present since the component was added to GOV.UK Frontend, and I believe it’s to ensure there’s a gap between GOV.UK and a product name. Remove this margin when the `__logotype` element is the last child of its descendent, so that the margin is only included when there is a product name to show.
Also, fix formatting to other fixes to match the same formatting used for previous releases, with a colon after the PR number.
36degrees
force-pushed
the
header-hover-focus-fixes
branch
from
756831a
to
65823aa
Compare
|
@hannalaakso I've changed the approach for the second fix, by removing the margin if the I think that addresses the issue you raised? |
There was a problem hiding this comment.
@36degrees Looks great 🎉 I opened #2181 for the other issues I spotted to do with the product name hover/focus states (nothing to do with the changes in this PR).
Address two minor issues with the hover and focus states for the GOV.UK logo (including product name, when present):
1. With product name
When the header includes a product name, the homepage link’s underline (on hover) and the focus state take up an additional 10px on the right hand side, because of the 10px right padding on
govuk-header__product-name.This padding is there to ensure there is a gap left between the product name and the menu button. Remove it, and add a 10px margin to
govuk-header__link--homepageinstead which achieves the same result but without adding an additional 10px to the hover and focus states.2. Without product name
When the header does not include a product name, the homepage link’s underline (on hover) and the focus state take up an additional 5px on the right hand side, because of a 5px margin on
govuk-header__logotype.This margin has been present since the component was added to GOV.UK Frontend, and I believe it’s to ensure there’s a gap between GOV.UK and a product name. Remove this margin when the
__logotypeelement is the last child of its descendent, so that the margin is only included when there is a product name to show.