-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add migration for sass padding and margin length values #7264
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
Conversation
Co-Authored-By: Aaron Casanova <[email protected]>
Co-Authored-By: Aaron Casanova <[email protected]>
|
/snapit |
|
🫰✨ Thanks @lgriffee! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected]yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
Co-Authored-By: Aaron Casanova <[email protected]>
Co-Authored-By: Aaron Casanova <[email protected]>
|
/snapit |
|
🫰✨ Thanks @lgriffee! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
Co-Authored-By: Aaron Casanova <[email protected]>
Co-Authored-By: Aaron Casanova <[email protected]>
Co-Authored-By: Aaron Casanova <[email protected]>
|
/snapit |
|
🫰✨ Thanks @lgriffee! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
sam-b-rose
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, migration is looking great! I like the more generic approach of replace-sass-lengths rather than just replace-sass-padding. Just a few comments and "thank yous" in the review 👍
polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts
Show resolved
Hide resolved
polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts
Outdated
Show resolved
Hide resolved
polaris-migrator/src/migrations/replace-sass-lengths/replace-sass-lengths.ts
Outdated
Show resolved
Hide resolved
polaris-migrator/src/migrations/replace-sass-lengths/tests/replace-sass-lengths.output.scss
Outdated
Show resolved
Hide resolved
Co-Authored-By: Aaron Casanova <[email protected]>
Co-Authored-By: Aaron Casanova <[email protected]>
|
/snapit |
|
🫰✨ Thanks @aaronccasanova! Your snapshots have been published to npm. Test the snapshots by updating your yarn add @shopify/[email protected]yarn add @shopify/[email protected] |
sam-b-rose
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks 💯 the migration PR in web is excellent, what a refactor! 👏
|
Why was it decided to replace |
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] ### Minor Changes - [#7264](#7264) [`5a1f45f7a`](5a1f45f) Thanks [@lgriffee](https://github.com/lgriffee)! - Add sass padding and margin migration ### Patch Changes - [#7315](#7315) [`c958899c7`](c958899) Thanks [@lgriffee](https://github.com/lgriffee)! - Remove `0` and `0px` length values from `replace-sass-lengths` migration - Updated dependencies \[[`0be40aa94`](0be40aa)]: - @shopify/[email protected] ## @shopify/[email protected] ### Minor Changes - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added alpha `Inline` component - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added `AlphaStack` component - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added `Columns` component - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added `Box` component - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added `AlphaCard` component - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added `fullWidth` prop to `AlphaStack` and updated styleguide docs - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added alpha `ContentBlock` component ### Patch Changes - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Updated `AlphaCard` border radius to a boolean - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `Columns` spacing - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `background` prop on `AlphaCard` for consistency - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Rename `Tiles` component - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `AlphaCard` `shadow` prop - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Refactored token types in primitive Layout components Exposed `DepthShadowAlias` type - [#7291](#7291) [`3941f5281`](3941f52) Thanks [@atesgoral](https://github.com/atesgoral)! - Improve default style class selection of `Avatar` by using the entire name instead of just the first letter - [#7332](#7332) [`2ee5c5d74`](2ee5c5d) Thanks [@mattkubej](https://github.com/mattkubej)! - `PopoverOverlay` closes focused `Popover` when pressing `Escape` - Updated dependencies \[[`0be40aa94`](0be40aa)]: - @shopify/[email protected] ## @shopify/[email protected] ### Minor Changes - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Refactored token types in primitive Layout components Exposed `DepthShadowAlias` type ## @shopify/[email protected] ### Patch Changes - Updated dependencies \[[`5a1f45f7a`](5a1f45f), [`c958899c7`](c958899)]: - @shopify/[email protected] ## @shopify/[email protected] ### Patch Changes - Updated dependencies \[[`0be40aa94`](0be40aa)]: - @shopify/[email protected] ## [email protected] ### Minor Changes - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added alpha pages for `AlphaStack`, `Box`, `Columns`, `Inline`, and `Tile` components. Updated `StatusBanner` to capitalize status value. - [#7353](#7353) [`558b1cfae`](558b1cf) Thanks [@kyledurand](https://github.com/kyledurand)! - Ported codesandbox init code to React 18 - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added alpha page for `ContentBlock` ### Patch Changes - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Updated `AlphaCard` border radius to a boolean - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `Columns` spacing - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `background` prop on `AlphaCard` for consistency - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Rename `Tiles` component - [#7312](#7312) [`2349db304`](2349db3) Thanks [@martenbjork](https://github.com/martenbjork)! - Added a redirect pointing to the Polaris license on Github - [#7343](#7343) [`7ced95bfb`](7ced95b) Thanks [@laurkim](https://github.com/laurkim)! - Updated alpha `Box` page to remove sentence regarding usage in existing components - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Renamed `AlphaCard` `shadow` prop - [#7056](#7056) [`0be40aa94`](0be40aa) Thanks [@laurkim](https://github.com/laurkim)! - Added `fullWidth` prop to `AlphaStack` and updated styleguide docs - [#7311](#7311) [`fc05e3152`](fc05e31) Thanks [@martenbjork](https://github.com/martenbjork)! - Improved the layout on mobile - Updated dependencies \[[`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`3941f5281`](3941f52), [`2ee5c5d74`](2ee5c5d)]: - @shopify/[email protected] - @shopify/[email protected] ## [email protected] ### Patch Changes - Updated dependencies \[[`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`0be40aa94`](0be40aa), [`3941f5281`](3941f52), [`2ee5c5d74`](2ee5c5d)]: - @shopify/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
@itwasmattgregg That's a fantastic question, and one which sparked a Discussion culminating in our decision to stick with literal That simplification will come in future migrations 👍 |
WHY are these changes introduced?
Part of #7212
WHAT is this pull request doing?
Adding a
replace-sass-lengthsmigration script.Running
npx @shopify/polaris-migrator replace-sass-lengths "**/*.scss"will target the followingpaddingandmarginproperties and replace anypx,remorrem()values that align with our spacing tokens.paddingpadding-toppadding-rightpadding-bottompadding-leftpadding-inlinepadding-inline-startpadding-inline-endpadding-blockpadding-block-startpadding-block-endmarginmargin-topmargin-rightmargin-bottommargin-leftmargin-inlinemargin-inline-startmargin-inline-endmargin-blockmargin-block-startmargin-block-end