Skip to content

Conversation

@aaronccasanova
Copy link
Member

@aaronccasanova aaronccasanova commented Jun 28, 2023

This PR restructures the Select component override selectors as a quick fix to a Sass compilation issue. We are still unpacking why the original selector is not compiling as expected, but here is what @sophschneider and I have discovered thus far:

Selector:#{$se23} &:hover:not(.disabled) .Backdrop

A single selector (.disabled) in the :not() pseudo-class selector list compiles as expected

Screenshot 2023-06-28 at 10 15 51 AM

Selector:#{$se23} &:hover:not(.disabled, .error) .Backdrop

Multiple selectors (.disabled, .error) in the :not() pseudo-class selector list completely omits :not() when compiled

Screenshot 2023-06-28 at 10 17 40 AM

Selector:#{$se23} &:hover:not(disabled) .Backdrop, #{$se23} &:hover:not(.error) .Backdrop

Splitting :not() into a separate grouped selector list compiles as expected with the tradeoff of doubling the override selector output. UPDATE: An improved pattern was found (see below).

Screenshot 2023-06-28 at 10 19 42 AM

There are a few areas of interest we intent to explore for resolving the underlying issue. However, I think the proposed fix and tradeoffs outlined above are an acceptable patch, addresses the issue observed in staging, and can be followed up with a proper solution when it is found.

UPDATE:

Selector:#{$se23} &:not(.disabled):not(.error)

Chaining :not() pseudo-classes compiles as expected and avoids the duplicate override selector output

image

@aaronccasanova aaronccasanova self-assigned this Jun 28, 2023
@aaronccasanova aaronccasanova marked this pull request as ready for review June 28, 2023 19:27
@aaronccasanova
Copy link
Member Author

/snapit

@aaronccasanova aaronccasanova added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Jun 28, 2023
@aaronccasanova
Copy link
Member Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @aaronccasanova! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-cli@0.0.0-snapshot-release-20230628195344
yarn add @shopify/polaris@0.0.0-snapshot-release-20230628195344

@aaronccasanova aaronccasanova merged commit c72d2f9 into main Jun 28, 2023
@aaronccasanova aaronccasanova deleted the restructure-select-overrides branch June 28, 2023 22:46
kyledurand pushed a commit that referenced this pull request Jun 30, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-icons@7.2.0

### Minor Changes

- [#9581](#9581)
[`991d9fe69`](991d9fe)
Thanks [@Rusty-UX](https://github.com/Rusty-UX)! - Added BoldMajor,
BoldMinor, IndentMajor, IndentMinor, ItalicMajor, ItalicMinor,
OrderedListMajor, OrderedListMinor, OutdentMajor, OutdentMinor,
TextColorMajor, TextColorMinor, UnderlineMajor, UnderlineMinor icons


- [#9580](#9580)
[`75f08f32c`](75f08f3)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added optimize
script

## @shopify/polaris@11.3.1

### Patch Changes

- [#9556](#9556)
[`c72d2f905`](c72d2f9)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! -
Restructured `Select` override selectors to patch Sass compilation issue


- [#9031](#9031)
[`795ae3782`](795ae37)
Thanks [@danbrady](https://github.com/danbrady)! - Added support for
`Avatar` being presentational

- Updated dependencies
\[[`991d9fe69`](991d9fe),
[`75f08f32c`](75f08f3)]:
    -   @shopify/polaris-icons@7.2.0

## @shopify/polaris-cli@0.2.10



## polaris.shopify.com@0.55.8

### Patch Changes

- Updated dependencies
\[[`c72d2f905`](c72d2f9),
[`795ae3782`](795ae37),
[`991d9fe69`](991d9fe),
[`75f08f32c`](75f08f3)]:
    -   @shopify/polaris@11.3.1
    -   @shopify/polaris-icons@7.2.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
This PR restructures the `Select` component override selectors as a
quick fix to a Sass compilation issue. We are still unpacking why the
original selector is not compiling as expected, but here is what @qt314
and I have discovered thus far:

**Selector:** ✅ `#{$se23} &:hover:not(.disabled) .Backdrop`

A single selector (`.disabled`) in the `:not()` pseudo-class selector
list compiles as expected

![Screenshot 2023-06-28 at 10 15 51
AM](https://github.com/Shopify/polaris/assets/32409546/35a89c74-90ce-4235-8ca6-1ecf50ac90f4)

**Selector:** ❌ `#{$se23} &:hover:not(.disabled, .error) .Backdrop`

Multiple selectors (`.disabled, .error`) in the `:not()` pseudo-class
selector list completely omits `:not()` when compiled

![Screenshot 2023-06-28 at 10 17 40
AM](https://github.com/Shopify/polaris/assets/32409546/0eece84a-b9d2-4716-87ec-1b97216f826e)

**Selector:** ✅ `#{$se23} &:hover:not(disabled) .Backdrop, #{$se23}
&:hover:not(.error) .Backdrop`

~Splitting `:not()` into a separate grouped selector list compiles as
expected with the tradeoff of doubling the override selector output.~
UPDATE: An improved pattern was found (see below).

![Screenshot 2023-06-28 at 10 19 42
AM](https://github.com/Shopify/polaris/assets/32409546/2820e8d6-bdb4-4507-8f8f-5b704c6f777b)

There are a few areas of interest we intent to explore for resolving the
underlying issue. However, I think the proposed fix and tradeoffs
outlined above are an acceptable patch, addresses the issue observed in
staging, and can be followed up with a proper solution when it is found.

UPDATE:

**Selector:** ✅ `#{$se23} &:not(.disabled):not(.error)`

Chaining `:not()` pseudo-classes compiles as expected and avoids the
duplicate override selector output


![image](https://github.com/Shopify/polaris/assets/32409546/c41b704e-770c-4698-b198-1cba7420439a)
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-icons@7.2.0

### Minor Changes

- [Shopify#9581](Shopify#9581)
[`991d9fe69`](Shopify@991d9fe)
Thanks [@Rusty-UX](https://github.com/Rusty-UX)! - Added BoldMajor,
BoldMinor, IndentMajor, IndentMinor, ItalicMajor, ItalicMinor,
OrderedListMajor, OrderedListMinor, OutdentMajor, OutdentMinor,
TextColorMajor, TextColorMinor, UnderlineMajor, UnderlineMinor icons


- [Shopify#9580](Shopify#9580)
[`75f08f32c`](Shopify@75f08f3)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added optimize
script

## @shopify/polaris@11.3.1

### Patch Changes

- [Shopify#9556](Shopify#9556)
[`c72d2f905`](Shopify@c72d2f9)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! -
Restructured `Select` override selectors to patch Sass compilation issue


- [Shopify#9031](Shopify#9031)
[`795ae3782`](Shopify@795ae37)
Thanks [@danbrady](https://github.com/danbrady)! - Added support for
`Avatar` being presentational

- Updated dependencies
\[[`991d9fe69`](Shopify@991d9fe),
[`75f08f32c`](Shopify@75f08f3)]:
    -   @shopify/polaris-icons@7.2.0

## @shopify/polaris-cli@0.2.10



## polaris.shopify.com@0.55.8

### Patch Changes

- Updated dependencies
\[[`c72d2f905`](Shopify@c72d2f9),
[`795ae3782`](Shopify@795ae37),
[`991d9fe69`](Shopify@991d9fe),
[`75f08f32c`](Shopify@75f08f3)]:
    -   @shopify/polaris@11.3.1
    -   @shopify/polaris-icons@7.2.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
This PR restructures the `Select` component override selectors as a
quick fix to a Sass compilation issue. We are still unpacking why the
original selector is not compiling as expected, but here is what @qt314
and I have discovered thus far:

**Selector:** ✅ `#{$se23} &:hover:not(.disabled) .Backdrop`

A single selector (`.disabled`) in the `:not()` pseudo-class selector
list compiles as expected

![Screenshot 2023-06-28 at 10 15 51
AM](https://github.com/Shopify/polaris/assets/32409546/35a89c74-90ce-4235-8ca6-1ecf50ac90f4)

**Selector:** ❌ `#{$se23} &:hover:not(.disabled, .error) .Backdrop`

Multiple selectors (`.disabled, .error`) in the `:not()` pseudo-class
selector list completely omits `:not()` when compiled

![Screenshot 2023-06-28 at 10 17 40
AM](https://github.com/Shopify/polaris/assets/32409546/0eece84a-b9d2-4716-87ec-1b97216f826e)

**Selector:** ✅ `#{$se23} &:hover:not(disabled) .Backdrop, #{$se23}
&:hover:not(.error) .Backdrop`

~Splitting `:not()` into a separate grouped selector list compiles as
expected with the tradeoff of doubling the override selector output.~
UPDATE: An improved pattern was found (see below).

![Screenshot 2023-06-28 at 10 19 42
AM](https://github.com/Shopify/polaris/assets/32409546/2820e8d6-bdb4-4507-8f8f-5b704c6f777b)

There are a few areas of interest we intent to explore for resolving the
underlying issue. However, I think the proposed fix and tradeoffs
outlined above are an acceptable patch, addresses the issue observed in
staging, and can be followed up with a proper solution when it is found.

UPDATE:

**Selector:** ✅ `#{$se23} &:not(.disabled):not(.error)`

Chaining `:not()` pseudo-classes compiles as expected and avoids the
duplicate override selector output


![image](https://github.com/Shopify/polaris/assets/32409546/c41b704e-770c-4698-b198-1cba7420439a)
ascherkus pushed a commit to ascherkus/polaris that referenced this pull request Feb 19, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-icons@7.2.0

### Minor Changes

- [Shopify#9581](Shopify#9581)
[`991d9fe69`](Shopify@a847d9c)
Thanks [@Rusty-UX](https://github.com/Rusty-UX)! - Added BoldMajor,
BoldMinor, IndentMajor, IndentMinor, ItalicMajor, ItalicMinor,
OrderedListMajor, OrderedListMinor, OutdentMajor, OutdentMinor,
TextColorMajor, TextColorMinor, UnderlineMajor, UnderlineMinor icons


- [Shopify#9580](Shopify#9580)
[`75f08f32c`](Shopify@01fb273)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added optimize
script

## @shopify/polaris@11.3.1

### Patch Changes

- [Shopify#9556](Shopify#9556)
[`c72d2f905`](Shopify@0c270b0)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! -
Restructured `Select` override selectors to patch Sass compilation issue


- [Shopify#9031](Shopify#9031)
[`795ae3782`](Shopify@c68e956)
Thanks [@danbrady](https://github.com/danbrady)! - Added support for
`Avatar` being presentational

- Updated dependencies
\[[`991d9fe69`](Shopify@a847d9c),
[`75f08f32c`](Shopify@01fb273)]:
    -   @shopify/polaris-icons@7.2.0

## @shopify/polaris-cli@0.2.10



## polaris.shopify.com@0.55.8

### Patch Changes

- Updated dependencies
\[[`c72d2f905`](Shopify@0c270b0),
[`795ae3782`](Shopify@c68e956),
[`991d9fe69`](Shopify@a847d9c),
[`75f08f32c`](Shopify@01fb273)]:
    -   @shopify/polaris@11.3.1
    -   @shopify/polaris-icons@7.2.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖Skip Changelog Causes CI to ignore changelog update check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants