[Layout foundations] Add support for LegacyCard in Card styles #8373

laurkim · 2023-02-15T17:33:20Z

WHY are these changes introduced?

As we're migrating usage of Card to LegacyCard, there are cases where components are nested as Card + LegacyCard so classes aren't getting properly applied in this use case and causing visual regressions.

Card with missing spacing

WHAT is this pull request doing?

Adds support for nested LegacyCard or Card classes in LegacyCard.scss and Card.scss.
Spin instance

Fixed card

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

Tested on mobile
Tested on multiple browsers
Tested for accessibility
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide

laurkim · 2023-02-15T17:33:53Z

/snapit

github-actions · 2023-02-15T17:38:08Z

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

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

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

github-actions · 2023-02-15T17:46:31Z

size-limit report 📦

Path	Size
polaris-react-cjs	220.12 KB (0%)
polaris-react-esm	139.98 KB (0%)
polaris-react-esnext	195.91 KB (+0.03% 🔺)
polaris-react-css	42.41 KB (+0.14% 🔺)

laurkim · 2023-02-15T17:51:57Z

/snapit

github-actions · 2023-02-15T17:56:12Z

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

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

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

laurkim · 2023-02-15T18:03:25Z

/snapit

github-actions · 2023-02-15T18:07:33Z

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

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

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

yarn add @shopify/[email protected]

kyledurand

Code and 🎩 Look good. Can we add an issue somewhere if you haven't already (maybe in the v11 main issue) to remove this code once Card has been removed?

laurkim · 2023-02-15T18:36:06Z

Code and 🎩 Look good. Can we add an issue somewhere if you haven't already (maybe in the v11 main issue) to remove this code once Card has been removed?

@aveline created an issue here 🙌

@alex-page

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/[email protected] ### Major Changes - [#8360](#8360) [`fd28b3a7a`](fd28b3a) Thanks [@alex-page](https://github.com/alex-page)! - Disallows percentage and other CSS values as a unit ## @shopify/[email protected] ### Minor Changes - [#8337](#8337) [`8a15048e3`](8a15048) Thanks [@alex-page](https://github.com/alex-page)! - Rename migrations so they are prefixed with the relevant version number ### Patch Changes - Updated dependencies \[[`fd28b3a7a`](fd28b3a)]: - @shopify/[email protected] ## @shopify/[email protected] ### Patch Changes - Updated dependencies \[[`8a15048e3`](8a15048)]: - @shopify/[email protected] ## @shopify/[email protected] ### Patch Changes - [#8373](#8373) [`684881b68`](684881b) Thanks [@laurkim](https://github.com/laurkim)! - Added support for `LegacyCard` in `Card` classes - [#8344](#8344) [`61f4e9254`](61f4e92) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed broken links in storybook and documentation examples ## [email protected] ### Patch Changes - [#8344](#8344) [`61f4e9254`](61f4e92) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed broken links in storybook and documentation examples - Updated dependencies \[[`684881b68`](684881b), [`61f4e9254`](61f4e92)]: - @shopify/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…fy#8373) ### WHY are these changes introduced? As we're migrating usage of Card to LegacyCard, there are cases where components are nested as `Card` + `LegacyCard` so classes aren't getting properly applied in this use case and causing visual regressions. <details> <summary>Card with missing spacing</summary> <img src="https://user-images.githubusercontent.com/26749317/219107735-a6af62d2-f21c-40c1-b271-55d2debcb967.png" alt="Card with missing spacing"> </details> ### WHAT is this pull request doing? Adds support for nested LegacyCard or Card classes in `LegacyCard.scss` and `Card.scss`. [Spin instance](https://admin.web.fix-classes-foundations.lo-kim.us.spin.dev/store/shop1/products/1) <details> <summary>Fixed card</summary> <img src="https://user-images.githubusercontent.com/26749317/219118069-371575c4-9720-455e-b124-343a667ad811.png" alt="Fixed card"> </details>  ### 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)  <details> <summary>Copy-paste this code in <code>playground/Playground.tsx</code>:</summary> ```jsx import React from 'react'; import {Page} from '../src'; export function Playground() { return ( <Page title="Playground"> {/* Add the code you want to test in here */} </Page> ); } ``` </details> ### 🎩 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) - [ ] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide

@alex-page

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/[email protected] ### Major Changes - [Shopify#8360](Shopify#8360) [`fd28b3a7a`](Shopify@fd28b3a) Thanks [@alex-page](https://github.com/alex-page)! - Disallows percentage and other CSS values as a unit ## @shopify/[email protected] ### Minor Changes - [Shopify#8337](Shopify#8337) [`8a15048e3`](Shopify@8a15048) Thanks [@alex-page](https://github.com/alex-page)! - Rename migrations so they are prefixed with the relevant version number ### Patch Changes - Updated dependencies \[[`fd28b3a7a`](Shopify@fd28b3a)]: - @shopify/[email protected] ## @shopify/[email protected] ### Patch Changes - Updated dependencies \[[`8a15048e3`](Shopify@8a15048)]: - @shopify/[email protected] ## @shopify/[email protected] ### Patch Changes - [Shopify#8373](Shopify#8373) [`684881b68`](Shopify@684881b) Thanks [@laurkim](https://github.com/laurkim)! - Added support for `LegacyCard` in `Card` classes - [Shopify#8344](Shopify#8344) [`61f4e9254`](Shopify@61f4e92) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed broken links in storybook and documentation examples ## [email protected] ### Patch Changes - [Shopify#8344](Shopify#8344) [`61f4e9254`](Shopify@61f4e92) Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed broken links in storybook and documentation examples - Updated dependencies \[[`684881b68`](Shopify@684881b), [`61f4e9254`](Shopify@61f4e92)]: - @shopify/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

laurkim requested a review from aveline as a code owner February 15, 2023 17:33

laurkim self-assigned this Feb 15, 2023

laurkim added #gsd:30116 Bug Something is broken and not working as intended in the system. labels Feb 15, 2023

laurkim requested a review from kyledurand February 15, 2023 18:20

laurkim added 2 commits February 15, 2023 13:21

[Layout foundations] Add support for LegacyCard in Card styles

3833a3a

Add changeset

f0e2361

laurkim force-pushed the lo/add-support-for-legacy-card branch from c129546 to f0e2361 Compare February 15, 2023 18:22

aveline approved these changes Feb 15, 2023

View reviewed changes

kyledurand approved these changes Feb 15, 2023

View reviewed changes

laurkim merged commit 684881b into main Feb 15, 2023

laurkim deleted the lo/add-support-for-legacy-card branch February 15, 2023 18:46

shopify-github-actions-access bot mentioned this pull request Feb 15, 2023

Version Packages #8361

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Layout foundations] Add support for LegacyCard in Card styles #8373

[Layout foundations] Add support for LegacyCard in Card styles #8373

laurkim commented Feb 15, 2023 •

edited

Loading

laurkim commented Feb 15, 2023

github-actions bot commented Feb 15, 2023

github-actions bot commented Feb 15, 2023 •

edited

Loading

laurkim commented Feb 15, 2023

github-actions bot commented Feb 15, 2023

laurkim commented Feb 15, 2023

github-actions bot commented Feb 15, 2023

kyledurand left a comment

laurkim commented Feb 15, 2023

[Layout foundations] Add support for LegacyCard in Card styles #8373

[Layout foundations] Add support for LegacyCard in Card styles #8373

Conversation

laurkim commented Feb 15, 2023 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to 🎩

🎩 checklist

laurkim commented Feb 15, 2023

github-actions bot commented Feb 15, 2023

github-actions bot commented Feb 15, 2023 • edited Loading

size-limit report 📦

laurkim commented Feb 15, 2023

github-actions bot commented Feb 15, 2023

laurkim commented Feb 15, 2023

github-actions bot commented Feb 15, 2023

kyledurand left a comment

Choose a reason for hiding this comment

laurkim commented Feb 15, 2023

laurkim commented Feb 15, 2023 •

edited

Loading

github-actions bot commented Feb 15, 2023 •

edited

Loading