-
Notifications
You must be signed in to change notification settings - Fork 331
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
Ensure footer links look clickable #1672
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-1672
December 4, 2019 11:17
Inactive
From @timpaul :
|
NickColley
force-pushed
the
add-underlines-to-footer-component-links
branch
from
December 9, 2019 16:09
efbebba
to
690c1f9
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-1672
December 9, 2019 16:09
Inactive
36degrees
approved these changes
Dec 12, 2019
We originally removed the underlines from these links to make them consistent with the approach we have for the header component. In order to make sure these links look like links in this component I have reverted this change. Original reasoning for removing underlines: " I believe the intention was to treat the links in the footer in the same way we treat other navigational elements – like the header, or in the case of the Design System the side navigation. Where the ‘thing’ is a ‘list of links’, the thinking is/was that they don’t need the underlining because there’s no wider context in which they need to be distinguished. The exception is any link that does appear within e.g. a paragraph, in which case it does need the underline to distinguish it. It has been questioned a few times on support recently though. The benefit, I believe, is that it allows us to have a clear hover state for those links, which we didn’t previously have "
NickColley
force-pushed
the
add-underlines-to-footer-component-links
branch
from
December 12, 2019 13:58
690c1f9
to
a552760
Compare
👏 |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We originally removed the underlines from these links to make them consistent with the approach we have for the header component.
Many community members have asked why we have made this change, and justification seems to be related to allowing a better hover state.
This makes sense but is not consistent with our other components, for example:
do not have a distinct hover state.
I think we should consider hover states (and touch states) at a system level separately.
In the meantime we can make sure that the links in the footer have the affordance of links, as we did not have any evidence from users that removing these underlines is helpful.
Original reasoning for removing underlines: #1321 (comment)
Screenshots
Before without underlines
After with underlines