Skip to content
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

Replace bg-secondary-experimental with bg-subdued for Tables #10649

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

mattkubej
Copy link
Contributor

@mattkubej mattkubej commented Sep 22, 2023

WHY are these changes introduced?

A previous change 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.

🎩 checklist

@mattkubej
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @mattkubej! 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-20230922191018
yarn add @shopify/polaris@0.0.0-snapshot-release-20230922191018
yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230922191018
yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230922191018

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.

Checked Storybook and each look good to me 👍

@mattkubej mattkubej merged commit 3bafbea into main Sep 22, 2023
14 checks passed
@mattkubej mattkubej deleted the tables/remove-bg-secondary branch September 22, 2023 21:07
sophschneider pushed a commit that referenced this pull request Sep 25, 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-tokens@7.11.0

### Minor Changes

- [#10647](#10647)
[`f26cebe99`](f26cebe)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Added
composite `text` and `font-letter-spacing` tokens


- [#10643](#10643)
[`2359e5f5a`](2359e5f)
Thanks [@lgriffee](https://github.com/lgriffee)! - Added semantic
`space` token scale

## @shopify/polaris-migrator@0.22.6

### Patch Changes

- Updated dependencies
\[[`f26cebe99`](f26cebe),
[`2359e5f5a`](2359e5f)]:
    -   @shopify/polaris-tokens@7.11.0
    -   @shopify/stylelint-polaris@14.0.6

## @shopify/polaris@11.20.1

### Patch Changes

- [#10665](#10665)
[`22dbada34`](22dbada)
Thanks [@chloerice](https://github.com/chloerice)! - Fixed `Modal`
missing `FrameContext` error


- [#10663](#10663)
[`f6a965102`](f6a9651)
Thanks [@chloerice](https://github.com/chloerice)! - Reverted disabling
of `Page` `secondaryActions` when `IndexFilters` is in filter `mode`


- [#10649](#10649)
[`3bafbea50`](3bafbea)
Thanks [@mattkubej](https://github.com/mattkubej)! - Replace usages of
`bg-secondary-experimental` with `bg-subdued` for Tables

- Updated dependencies
\[[`f26cebe99`](f26cebe),
[`2359e5f5a`](2359e5f)]:
    -   @shopify/polaris-tokens@7.11.0

## @shopify/stylelint-polaris@14.0.6

### Patch Changes

- Updated dependencies
\[[`f26cebe99`](f26cebe),
[`2359e5f5a`](2359e5f)]:
    -   @shopify/polaris-tokens@7.11.0

## polaris-for-vscode@0.8.0

### Minor Changes

- [#10647](#10647)
[`f26cebe99`](f26cebe)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Added
composite `text` token completions

## polaris.shopify.com@0.57.9

### Patch Changes

- [#10609](#10609)
[`b53189c72`](b53189c)
Thanks [@kyledurand](https://github.com/kyledurand)! - Updated component
example backgrounds

- Updated dependencies
\[[`22dbada34`](22dbada),
[`f26cebe99`](f26cebe),
[`f6a965102`](f6a9651),
[`3bafbea50`](3bafbea),
[`2359e5f5a`](2359e5f)]:
    -   @shopify/polaris@11.20.1
    -   @shopify/polaris-tokens@7.11.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
…#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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants