-
Notifications
You must be signed in to change notification settings - Fork 20
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 Sass warning for extending a compound selector #1933
Conversation
27ffe22
to
77b0e5f
Compare
77b0e5f
to
392c173
Compare
@@ -34,6 +34,21 @@ $large-input-size: 50px; | |||
} | |||
} | |||
|
|||
%gem-c-search-input-focus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funnily, it seems like this is what I had originally done in the PR that introduced this warning, but it was suggested to extend the original selector instead 😆
#1752 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danacotoran ah ha that's good to see. It looks like it was very hard to anticipate that this would have the additional effects it did (I thought I was only removing a warning when I looked at this, I had no idea it produced 59 extra code outputs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for picking this up Kevin.
This replaces the extending of a compound selector as this causes a warning: ``` WARNING on line 80, column 13 of /Users/kevindew/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/govuk_publishing_components-24.1.1/app/assets/stylesheets/govuk_publishing_components/components/_search.scss: Compound selectors may no longer be extended. Consider `@extend .gem-c-search__input, [type="search"], :focus` instead. See http://bit.ly/ExtendCompound for details. ``` In order to share this code I've added a placeholder so it can be included in both places. When comparing the before and after of this there is quite a lot of changes to output CSS. With the previous version of this code there were 59 references to "#global-header .gem-c-search__input" compared to 1 afterwards. ``` cat public/assets/collections/application-d3690d26e75f4569773cce03d0dfe714cb2e6f649923fa256791cc92b48122b9.css | grep '#global-header .gem-c-search__input' | wc -l 59 cat public/assets/collections/application-c8c98d4348c02a0206805a873fcf4ba0f18d72e87ba14908cd4a20d1d9659969.css | grep '#global-header .gem-c-search__input' | wc -l 1 ``` I think these 58 references that are removed are mostly accidental references that were never intended to be there (see examples below) however it does make me a bit nervous that this change may do more than is expected. Some examples of CSS that is no longer generated: ``` -- .gem-c-step-nav__list-item--active .gem-c-step-nav__link:link:focus, .gem-c-step-nav__list-item--active #global-header .gem-c-search__input[type="search"]:link:focus, #global-header .gem-c-step-nav__list-item--active .gem-c-search__input[type="search"]:link:focus { color: #0b0c0c; } -- .gem-c-step-nav__list-item--active .gem-c-step-nav__link:link:focus, .gem-c-step-nav__list-item--active #global-header .gem-c-search__input[type="search"]:link:focus, #global-header .gem-c-step-nav__list-item--active .gem-c-search__input[type="search"]:link:focus { color: #000000; } } -- .app-c-topic-list__link--no-underline:hover:focus, #global-header .gem-c-search__input[type="search"]:hover:focus { text-decoration: none; } -- .youtube__video-wrapper--inverse .govuk-link:link:focus, .youtube__video-wrapper--inverse #global-header .gem-c-search__input[type="search"]:link:focus, #global-header .youtube__video-wrapper--inverse .gem-c-search__input[type="search"]:link:focus { color: #0b0c0c; } ```
392c173
to
683c7c0
Compare
This replaces the extending of a compound selector as this causes a
warning:
In order to share this code I've added a placeholder so it can be
included in both places.
When comparing the before and after of this there is quite a lot of
changes to output CSS. With the previous version of this code there were
59 references to "#global-header .gem-c-search__input" compared to 1
afterwards.
I think these 58 references that are removed are mostly accidental
references that were never intended to be there (see examples below)
however it does make me a bit nervous that this change may do more than
is expected.
Some examples of CSS that is no longer generated: