Skip to content
This repository was archived by the owner on Sep 30, 2025. It is now read-only.

Conversation

mrcthms
Copy link
Contributor

@mrcthms mrcthms commented Sep 20, 2023

WHY are these changes introduced?

Linked to https://github.com/Shopify/web/issues/104248

We have noticed that the focus states for the actions in the Page Header do not match the focus states for other buttons within Polaris. They have a darker background, and an inset shadow. This should be the :active state, not the :focus-visible state.

WHAT is this pull request doing?

This PR is updating the visuals of the actions within the Page Header, namely the breadcrumbs and actions.

  • We set the :focus style to match the :hover style, to bring this in line with other Button components.
  • We include new :focus-visible style rules to remove the box shadow for the page actions
  • We change the existing :focus styles to :active, as these styles should be applied when the page actions are pressed

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Storybook: https://5d559397bae39100201eedc1-kvktrdbvcr.chromatic.com/?path=/story/all-components-page--default&globals=polarisSummerEditions2023:true

🎩 checklist

@mrcthms
Copy link
Contributor Author

mrcthms commented Sep 20, 2023

/snapit


/* stylelint-disable-next-line selector-max-combinators -- SE23 match button group */
&:focus-visible {
background-color: var(--p-color-bg-strong-hover);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worthwhile making a single selector for &:hover and &focus-visible, so they share the same style? Seems like we'd probably want to keep these in sync.

}

&:focus {
background-color: var(--p-color-bg-strong);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be suffixed with -hover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, this is a leftover artefact from debugging – it has no discernible effect due to specificity, so I will remove.

@mrcthms
Copy link
Contributor Author

mrcthms commented Sep 21, 2023

/snapit

@github-actions
Copy link
Contributor

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

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

yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230921143710
yarn add @shopify/polaris@0.0.0-snapshot-release-20230921143710
yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230921143710
yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230921143710

@mrcthms
Copy link
Contributor Author

mrcthms commented Sep 21, 2023

/snapit

@github-actions
Copy link
Contributor

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

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

yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230921144717
yarn add @shopify/polaris@0.0.0-snapshot-release-20230921144717
yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230921144717
yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230921144717

@ouellettejordan ouellettejordan self-requested a review September 21, 2023 19:39
Copy link

@ouellettejordan ouellettejordan left a comment

Choose a reason for hiding this comment

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

Tested in Storybook and looks good 👍

@mrcthms mrcthms merged commit 3efbc1b into main Sep 22, 2023
@mrcthms mrcthms deleted the mrcthms-no-focus-button-shadow branch September 22, 2023 10:23
laurkim pushed a commit that referenced this pull request Sep 22, 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@11.20.0

### Minor Changes

- [#10477](#10477)
[`790a001cd`](790a001)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
semantic `color` tokens


- [#10478](#10478)
[`8be227e0c`](8be227e)
Thanks [@MaxCloutier](https://github.com/MaxCloutier)! - Added
`allowFiltering` prop on `ActionList`, and `filterActions` prop on Page
Header


- [#9445](#9445)
[`7be9c243a`](7be9c24)
Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added
support for subheaders and selection of a range of `IndexTable.Rows` --
See the [With
subheaders](https://polaris.shopify.com/components/tables/index-table)
example on polaris.shopify.com for how to properly configure
    -   `IndexTable.Row`
- Added support for setting the `indeterminate` value on the `selected`
prop
- Added the `selectionRange` prop to specify a range of other
consecutive, related rows selected when the row is selected
- Added the `rowType` prop to indicate the relationship or role of the
row's contents (defaults to `data`, `subheader` renders the row to look
and behave like the table header row)
Added support for setting accessibility attributes on `IndexTable.Cell`
    -   `IndexTable.Cell`
- Added the `as` prop to support rendering the cell as a `th` element if
it is serving as a subheading cell
- Added support for the `headers` attribute to manually associate all
headers when the cell is described by more than its column heading
- Added support for the `colSpan` attribute to specify the number of the
columns that the cell element should extend to
- Added support for the `scope` attribute to indicate whether the `th`
is a header for a column, row, or group of columns or rows


- [#10490](#10490)
[`863f15ff2`](863f15f)
Thanks [@mrcthms](https://github.com/mrcthms)! - Add new
`IndexFiltersManager` for allowing disabling of Page Header actions when
in Filtering or EditingColumns mode


- [#10566](#10566)
[`9fed74317`](9fed743)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed a bug in
`Filters` where changes to the `appliedFilters` prop were not being
handled

### Patch Changes

- [#10404](#10404)
[`5acfcec04`](5acfcec)
Thanks [@jesstelford](https://github.com/jesstelford)! - Scoped CSS
variables for non-responsive props on `Tooltip`, `RangeSlider`,
`ProgressBar`, and `HorizontalStack`.


- [#10582](#10582)
[`3efbc1b4e`](3efbc1b)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed the focus states
of actions within the Page Header component


- [#10492](#10492)
[`d5ff72dec`](d5ff72d)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated Storybook
stories to be wrapped with an empty FrameContext.Provider

- Updated dependencies
\[[`fe1aac1b5`](fe1aac1),
[`790a001cd`](790a001),
[`63cf3ad24`](63cf3ad),
[`120e96eae`](120e96e)]:
    -   @shopify/polaris-tokens@7.10.0

## @shopify/polaris-tokens@7.10.0

### Minor Changes

- [#10465](#10465)
[`fe1aac1b5`](fe1aac1)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
private primitive `colors`


- [#10477](#10477)
[`790a001cd`](790a001)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
semantic `color` tokens


- [#10600](#10600)
[`63cf3ad24`](63cf3ad)
Thanks [@lgriffee](https://github.com/lgriffee)! - Added public
primitive and semantic `shadow` token scales

### Patch Changes

- [#10485](#10485)
[`120e96eae`](120e96e)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated public
primitive `base` and `light-uplift` theme scales

## @shopify/polaris-migrator@0.22.5

### Patch Changes

- [#10575](#10575)
[`ea6b54284`](ea6b542)
Thanks [@aveline](https://github.com/aveline)! - Handled `buttonFrom`
and `buttonsFrom` functions in `Button` migration

- Updated dependencies
\[[`fe1aac1b5`](fe1aac1),
[`790a001cd`](790a001),
[`63cf3ad24`](63cf3ad),
[`120e96eae`](120e96e)]:
    -   @shopify/polaris-tokens@7.10.0
    -   @shopify/stylelint-polaris@14.0.5

## @shopify/stylelint-polaris@14.0.5

### Patch Changes

- Updated dependencies
\[[`fe1aac1b5`](fe1aac1),
[`790a001cd`](790a001),
[`63cf3ad24`](63cf3ad),
[`120e96eae`](120e96e)]:
    -   @shopify/polaris-tokens@7.10.0

## polaris.shopify.com@0.57.8

### Patch Changes

- [#10605](#10605)
[`9748b0838`](9748b08)
Thanks [@laurkim](https://github.com/laurkim)! - Updated logic for
rendering `color` custom property previews in `TokenList`


- [#10573](#10573)
[`da09e0b8c`](da09e0b)
Thanks [@kyledurand](https://github.com/kyledurand)! - Updated copy url
to change browser url

- Updated dependencies
\[[`fe1aac1b5`](fe1aac1),
[`5acfcec04`](5acfcec),
[`790a001cd`](790a001),
[`8be227e0c`](8be227e),
[`63cf3ad24`](63cf3ad),
[`7be9c243a`](7be9c24),
[`863f15ff2`](863f15f),
[`3efbc1b4e`](3efbc1b),
[`d5ff72dec`](d5ff72d),
[`120e96eae`](120e96e),
[`9fed74317`](9fed743)]:
    -   @shopify/polaris-tokens@7.10.0
    -   @shopify/polaris@11.20.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
…er (Shopify#10582)

### WHY are these changes introduced?

Linked to Shopify/web#104248

We have noticed that the focus states for the actions in the Page Header
do not match the focus states for other buttons within Polaris. They
have a darker background, and an inset shadow. This should be the
`:active` state, not the `:focus-visible` state.

### WHAT is this pull request doing?

This PR is updating the visuals of the actions within the Page Header,
namely the breadcrumbs and actions.

- We set the `:focus` style to match the `:hover` style, to bring this
in line with other Button components.
- We include new `:focus-visible` style rules to remove the box shadow
for the page actions
- We change the existing `:focus` styles to `:active`, as these styles
should be applied when the page actions are pressed


### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

Storybook:
https://5d559397bae39100201eedc1-kvktrdbvcr.chromatic.com/?path=/story/all-components-page--default&globals=polarisSummerEditions2023:true


### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [x] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
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@11.20.0

### Minor Changes

- [Shopify#10477](Shopify#10477)
[`790a001cd`](Shopify@be6914d)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
semantic `color` tokens


- [Shopify#10478](Shopify#10478)
[`8be227e0c`](Shopify@b588f20)
Thanks [@MaxCloutier](https://github.com/MaxCloutier)! - Added
`allowFiltering` prop on `ActionList`, and `filterActions` prop on Page
Header


- [Shopify#9445](Shopify#9445)
[`7be9c243a`](Shopify@4069237)
Thanks [@m4thieulavoie](https://github.com/m4thieulavoie)! - Added
support for subheaders and selection of a range of `IndexTable.Rows` --
See the [With
subheaders](https://polaris.shopify.com/components/tables/index-table)
example on polaris.shopify.com for how to properly configure
    -   `IndexTable.Row`
- Added support for setting the `indeterminate` value on the `selected`
prop
- Added the `selectionRange` prop to specify a range of other
consecutive, related rows selected when the row is selected
- Added the `rowType` prop to indicate the relationship or role of the
row's contents (defaults to `data`, `subheader` renders the row to look
and behave like the table header row)
Added support for setting accessibility attributes on `IndexTable.Cell`
    -   `IndexTable.Cell`
- Added the `as` prop to support rendering the cell as a `th` element if
it is serving as a subheading cell
- Added support for the `headers` attribute to manually associate all
headers when the cell is described by more than its column heading
- Added support for the `colSpan` attribute to specify the number of the
columns that the cell element should extend to
- Added support for the `scope` attribute to indicate whether the `th`
is a header for a column, row, or group of columns or rows


- [Shopify#10490](Shopify#10490)
[`863f15ff2`](Shopify@07246bc)
Thanks [@mrcthms](https://github.com/mrcthms)! - Add new
`IndexFiltersManager` for allowing disabling of Page Header actions when
in Filtering or EditingColumns mode


- [Shopify#10566](Shopify#10566)
[`9fed74317`](Shopify@3fb446f)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed a bug in
`Filters` where changes to the `appliedFilters` prop were not being
handled

### Patch Changes

- [Shopify#10404](Shopify#10404)
[`5acfcec04`](Shopify@5cdd787)
Thanks [@jesstelford](https://github.com/jesstelford)! - Scoped CSS
variables for non-responsive props on `Tooltip`, `RangeSlider`,
`ProgressBar`, and `HorizontalStack`.


- [Shopify#10582](Shopify#10582)
[`3efbc1b4e`](Shopify@d47089b)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed the focus states
of actions within the Page Header component


- [Shopify#10492](Shopify#10492)
[`d5ff72dec`](Shopify@8057862)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated Storybook
stories to be wrapped with an empty FrameContext.Provider

- Updated dependencies
\[[`fe1aac1b5`](Shopify@a4f7ed7),
[`790a001cd`](Shopify@be6914d),
[`63cf3ad24`](Shopify@d4e6bd4),
[`120e96eae`](Shopify@fe98b0b)]:
    -   @shopify/polaris-tokens@7.10.0

## @shopify/polaris-tokens@7.10.0

### Minor Changes

- [Shopify#10465](Shopify#10465)
[`fe1aac1b5`](Shopify@a4f7ed7)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
private primitive `colors`


- [Shopify#10477](Shopify#10477)
[`790a001cd`](Shopify@be6914d)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Updated
semantic `color` tokens


- [Shopify#10600](Shopify#10600)
[`63cf3ad24`](Shopify@d4e6bd4)
Thanks [@lgriffee](https://github.com/lgriffee)! - Added public
primitive and semantic `shadow` token scales

### Patch Changes

- [Shopify#10485](Shopify#10485)
[`120e96eae`](Shopify@fe98b0b)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated public
primitive `base` and `light-uplift` theme scales

## @shopify/polaris-migrator@0.22.5

### Patch Changes

- [Shopify#10575](Shopify#10575)
[`ea6b54284`](Shopify@9761c8a)
Thanks [@aveline](https://github.com/aveline)! - Handled `buttonFrom`
and `buttonsFrom` functions in `Button` migration

- Updated dependencies
\[[`fe1aac1b5`](Shopify@a4f7ed7),
[`790a001cd`](Shopify@be6914d),
[`63cf3ad24`](Shopify@d4e6bd4),
[`120e96eae`](Shopify@fe98b0b)]:
    -   @shopify/polaris-tokens@7.10.0
    -   @shopify/stylelint-polaris@14.0.5

## @shopify/stylelint-polaris@14.0.5

### Patch Changes

- Updated dependencies
\[[`fe1aac1b5`](Shopify@a4f7ed7),
[`790a001cd`](Shopify@be6914d),
[`63cf3ad24`](Shopify@d4e6bd4),
[`120e96eae`](Shopify@fe98b0b)]:
    -   @shopify/polaris-tokens@7.10.0

## polaris.shopify.com@0.57.8

### Patch Changes

- [Shopify#10605](Shopify#10605)
[`9748b0838`](Shopify@a21aca4)
Thanks [@laurkim](https://github.com/laurkim)! - Updated logic for
rendering `color` custom property previews in `TokenList`


- [Shopify#10573](Shopify#10573)
[`da09e0b8c`](Shopify@439fedc)
Thanks [@kyledurand](https://github.com/kyledurand)! - Updated copy url
to change browser url

- Updated dependencies
\[[`fe1aac1b5`](Shopify@a4f7ed7),
[`5acfcec04`](Shopify@5cdd787),
[`790a001cd`](Shopify@be6914d),
[`8be227e0c`](Shopify@b588f20),
[`63cf3ad24`](Shopify@d4e6bd4),
[`7be9c243a`](Shopify@4069237),
[`863f15ff2`](Shopify@07246bc),
[`3efbc1b4e`](Shopify@d47089b),
[`d5ff72dec`](Shopify@8057862),
[`120e96eae`](Shopify@fe98b0b),
[`9fed74317`](Shopify@3fb446f)]:
    -   @shopify/polaris-tokens@7.10.0
    -   @shopify/polaris@11.20.0

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

Successfully merging this pull request may close these issues.

3 participants