Skip to content

Commit

Permalink
Fix Sass warning for extending a compound selector
Browse files Browse the repository at this point in the history
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;
}
```
  • Loading branch information
kevindew committed Feb 16, 2021
1 parent ba62d07 commit 27ffe22
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
useful summary for people upgrading their application, not a replication
of the commit log.

## Unreleased

* Fix Sass warning for extending a compound selector

## 24.2.0

* Update cookies banner to align it with govuk-frontend ([PR #1918](https://github.com/alphagov/govuk_publishing_components/pull/1918))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,21 @@ $large-input-size: 50px;
}
}

%gem-c-search__input-focus {
outline: $govuk-focus-width solid $govuk-focus-colour;
// Ensure outline appears outside of the element
outline-offset: 0;
// Double the border by adding its width again. Use `box-shadow` for this // instead of changing `border-width`
// Also, `outline` cannot be utilised here as it is already used for the yellow focus state.
box-shadow: inset 0 0 0 $govuk-border-width-form-element;

@include govuk-if-ie8 {
// IE8 doesn't support `box-shadow` so double the border with
// `border-width`.
border-width: $govuk-border-width-form-element * 2;
}
}

.gem-c-search__input[type="search"] { // overly specific to prevent some overrides from outside
@include govuk-font($size: 19, $line-height: (28 / 19));
margin: 0;
Expand All @@ -59,25 +74,14 @@ $large-input-size: 50px;
}

&:focus {
outline: $govuk-focus-width solid $govuk-focus-colour;
// Ensure outline appears outside of the element
outline-offset: 0;
// Double the border by adding its width again. Use `box-shadow` for this // instead of changing `border-width`
// Also, `outline` cannot be utilised here as it is already used for the yellow focus state.
box-shadow: inset 0 0 0 $govuk-border-width-form-element;

@include govuk-if-ie8 {
// IE8 doesn't support `box-shadow` so double the border with
// `border-width`.
border-width: $govuk-border-width-form-element * 2;
}
@extend %gem-c-search__input-focus;
}
}

@include govuk-compatibility(govuk_template) {
// ultra specific rule overrides focus styling from govuk_template
#global-header .gem-c-search__input[type="search"]:focus { // stylelint-disable selector-max-id
@extend .gem-c-search__input[type="search"]:focus; // stylelint-disable scss/at-extend-no-missing-placeholder
@extend %gem-c-search__input-focus;
}
}

Expand Down

0 comments on commit 27ffe22

Please sign in to comment.