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

Apply black text colour to link-back on :focus #544

Merged
merged 1 commit into from
Aug 15, 2017

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented Aug 8, 2017

What problem does the pull request solve?

Fixes issue #540 where a:link:focus in govuk_template applies text-link (blue) colour to this element

How has this been tested?

Supported browsers

Screenshots (if appropriate):

screen shot 2017-08-08 at 13 37 13

What type of change is it?

  • Bug fix (non-breaking change which fixes an issue)

Has the documentation been updated?

  • I have read the CONTRIBUTING document.

@gemmaleigh gemmaleigh temporarily deployed to govuk-elements-review-pr-544 August 8, 2017 12:38 Inactive
@@ -201,10 +201,12 @@ p,
&:link,
&:visited,
&:hover,
a#{&}:focus,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Author

@kr8n3r kr8n3r Aug 8, 2017

Choose a reason for hiding this comment

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

it evaluates the parent selector (.link-back) and outputs it after the a tag resulting in a.link-back:focus which neutralizes the a:link:focus set in govuk_template
introduced in sass 3.3

Copy link
Contributor

Choose a reason for hiding this comment

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

is &:focus not specific enough?

Copy link
Author

@kr8n3r kr8n3r Aug 8, 2017

Choose a reason for hiding this comment

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

nope :( it wouldn't be
screen shot 2017-08-08 at 15 04 00
(currently on gov.uk)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sad, specificity strikes again.

I've seen people double classes up too: '.link-back.link-back:focus'

Thanks for the extra info Jani 👍

@edwardhorsford
Copy link
Contributor

Nice one @igloosi.

I don't know if it's relevant, but the same issue doesn't appear to exist on the breadcrumbs:
screen shot 2017-08-08 at 19 23 05

@kr8n3r
Copy link
Author

kr8n3r commented Aug 9, 2017

there is but it's weird
screen shot 2017-08-09 at 09 59 32

if you look at the specificity it should on all items. but it only does on the last one
:magic

@gemmaleigh gemmaleigh merged commit dfb8f4b into master Aug 15, 2017
@36degrees 36degrees deleted the fix-link-back-focus branch September 19, 2017 10:29
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.

4 participants