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

Conversation

aaronccasanova
Copy link
Member

@aaronccasanova aaronccasanova commented Jun 21, 2023

Part of https://github.com/Shopify/polaris-summer-editions/issues/262

Closes https://github.com/Shopify/polaris-summer-editions/issues/646

  • Leveraged @shopify/polaris-migrator to swap bg-subdued and bg-secondary instances
  • Manually swapped bg-subdued and bg-secondary edge cases (after each migration)
  • Manually updated token swaps behind the beta flag

Example @shopify/polaris-migrator commands:

npx @shopify/polaris-migrator \
  styles-replace-custom-property \
  --from="--p-color-bg-secondary-experimental" \
  --to="--p-color-bg-secondary-experimental-temp" \
  "src/components/{Button,Card,LegacyCard,IndexTable,Tag,DataTable,KeyboardKey,Modal,ResourceList,Filters,LegacyFilters,Tabs,LegacyTabs}/**/*.scss"

npx @shopify/polaris-migrator \
  styles-replace-custom-property \
  --from="--p-color-bg-subdued" \
  --to="--p-color-bg-secondary-experimental" \
  "src/components/{Button,Card,LegacyCard,IndexTable,Tag,DataTable,KeyboardKey,Modal,ResourceList,Filters,LegacyFilters,Tabs,LegacyTabs}/**/*.scss"

npx @shopify/polaris-migrator \
  styles-replace-custom-property \
  --from="--p-color-bg-secondary-experimental-temp" \
  --to="--p-color-bg-subdued" \
  "src/components/{Button,Card,LegacyCard,IndexTable,Tag,DataTable,KeyboardKey,Modal,ResourceList,Filters,LegacyFilters,Tabs,LegacyTabs}/**/*.scss"

Example search after each migration:
VS Code search: ['"-]bg-subdued[^-]
Files to include: src/components/{Button,Card,LegacyCard,IndexTable,Tag,DataTable,KeyboardKey,Modal,ResourceList,Filters,LegacyFilters,Tabs,LegacyTabs}/**/*

@aaronccasanova aaronccasanova self-assigned this Jun 22, 2023
@aaronccasanova aaronccasanova added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Jun 22, 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-20230622163123
yarn add @shopify/polaris-codemods@0.0.0-snapshot-release-20230622163123
yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230622163123
yarn add @shopify/polaris@0.0.0-snapshot-release-20230622163123
yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230622163123
yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230622163123

@aaronccasanova aaronccasanova marked this pull request as ready for review June 22, 2023 18:22
Copy link
Member

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

Tophat looks good to me 👍

Copy link
Member

@sam-b-rose sam-b-rose left a comment

Choose a reason for hiding this comment

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

💯

@aaronccasanova aaronccasanova merged commit 8474a39 into main Jun 22, 2023
@aaronccasanova aaronccasanova deleted the swap-bg-tokens branch June 22, 2023 21:03
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
mattkubej added a commit that referenced this pull request Sep 22, 2023
### WHY are these changes introduced?

A [previous change](#9498) went
in to replace some instances of `bg-subdued` with
`bg-secondary-experimental`. There are now some inconsistencies between
our Table and List usages of these background colors. Additionally,
there is a migration planned for next week that will replace all
`bg-subdued` instances with `bg-secondary`.

### WHAT is this pull request doing?

To resolve the inconsistencies and prepare for the migration, this pull
request replaces instances of `bg-secondary-experiement` with the
`bg-subdued` token within `DataTable` and `IndexTable`.

NOTE: the existing usage of `bg-secondary-experiment` is not likely seen
visually, because header cells defined their background as `bg-subdued`
and sit on top of the row with `bg-secondary-experiment`. Likely further
improvement here, but this will combat the incorrect color from leaking
through.

### How to 🎩

Validate that `DataTable` and `IndexTable` do not render row backgrounds
with `secondary`, but with `subdued`.

- Storybook
-
[DataTable](https://storybook.web.swap-table-secondary-with-subdued.matt-kubej.us.spin.dev/?path=/story/all-components-datatable--default&globals=polarisSummerEditions2023:true;polarisSummerEditions2023ShadowBevelOptOut:true)
-
[IndexTable](https://storybook.web.swap-table-secondary-with-subdued.matt-kubej.us.spin.dev/?path=/story/all-components-indextable--default&globals=polarisSummerEditions2023:true;polarisSummerEditions2023ShadowBevelOptOut:true)
- Web
- [Product
Index](https://admin.web.swap-table-secondary-with-subdued.matt-kubej.us.spin.dev/store/shop1/products)
(using snapshot and loaded with products to test sticky header)

### 🎩 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
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
ascherkus pushed a commit to ascherkus/polaris that referenced this pull request Feb 19, 2025
…#10649)

### WHY are these changes introduced?

A [previous change](Shopify#9498) went
in to replace some instances of `bg-subdued` with
`bg-secondary-experimental`. There are now some inconsistencies between
our Table and List usages of these background colors. Additionally,
there is a migration planned for next week that will replace all
`bg-subdued` instances with `bg-secondary`.

### WHAT is this pull request doing?

To resolve the inconsistencies and prepare for the migration, this pull
request replaces instances of `bg-secondary-experiement` with the
`bg-subdued` token within `DataTable` and `IndexTable`.

NOTE: the existing usage of `bg-secondary-experiment` is not likely seen
visually, because header cells defined their background as `bg-subdued`
and sit on top of the row with `bg-secondary-experiment`. Likely further
improvement here, but this will combat the incorrect color from leaking
through.

### How to 🎩

Validate that `DataTable` and `IndexTable` do not render row backgrounds
with `secondary`, but with `subdued`.

- Storybook
-
[DataTable](https://storybook.web.swap-table-secondary-with-subdued.matt-kubej.us.spin.dev/?path=/story/all-components-datatable--default&globals=polarisSummerEditions2023:true;polarisSummerEditions2023ShadowBevelOptOut:true)
-
[IndexTable](https://storybook.web.swap-table-secondary-with-subdued.matt-kubej.us.spin.dev/?path=/story/all-components-indextable--default&globals=polarisSummerEditions2023:true;polarisSummerEditions2023ShadowBevelOptOut:true)
- Web
- [Product
Index](https://admin.web.swap-table-secondary-with-subdued.matt-kubej.us.spin.dev/store/shop1/products)
(using snapshot and loaded with products to test sticky header)

### 🎩 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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