Skip to content

Conversation

@gwyneplaine
Copy link
Contributor

@gwyneplaine gwyneplaine commented Sep 30, 2022

WHY are these changes introduced?
Part of #7211

WHAT is this pull request doing?
Adding a replace-sass-z-index migration script.

Running npx @shopify/polaris-migrator replace-sass-z-index "**/*.scss" will target z-index function usage and do the following:

  • If more than one argument exists we assume the code is invoking z-index to map-get on a custom sass map. In this case, we add a comment with a migration warning and code snippet replacing the legacy-polaris-v8.z-index usage with map-get
  • If there's a calculation in the same declaration but the legacy-polaris-v8.z-index invocation is valid and corresponds to a polaris token, we add a comment with a migration warning and a code snippet replacing that invocation with a css variable invocation pointing at the corresponding polaris-token.
  • If its just a valid legacy-polaris-v8.z-index invocation that corresponds to an existing polaris token, we replace it with a css-variable invocation pointing at the corresponding polaris-token.
  • Otherwise we ignore the declaration.

Things our codemod doesn't presently understand but should:

  • interpolations. calc(#{legacy-polaris-v8.z-index} + 1) for example is completely ignored by our code. As post-css interprets this as an invocation of a function called #{legacy-polaris-v8.z-index. It doesn't seem to understand interpolations at the moment. We can probably use something like style-lint's hasInterpolation to work around this, (ty @aaronccasanova), but not something we should worry about for first pass imo.

@gwyneplaine gwyneplaine changed the title initial migration mod for legacy-polaris-v8.z-index sass fn Add migration for sass z-index fn Sep 30, 2022
@gwyneplaine gwyneplaine self-assigned this Sep 30, 2022
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.

This looks 💯 great catch on the unnecessary ...node.nodes as well.

Feel free to /snapit and test and/or run on web 🚢

We can circle back for function reuse and clean up after the other length PR is merged.

@gwyneplaine gwyneplaine marked this pull request as ready for review October 3, 2022 22:33
@gwyneplaine gwyneplaine force-pushed the replace-sass-z-index branch from 74bc3cd to f3fa524 Compare October 3, 2022 22:43
@gwyneplaine
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

🫰✨ Thanks @gwyneplaine! 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]

@gwyneplaine
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @gwyneplaine! 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]

@gwyneplaine
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @gwyneplaine! 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]

@gwyneplaine
Copy link
Contributor Author

Added additional usecase of z-index([validKey], $fixed-element-stacking-order)
which contains a larger subset of our z-index tokens.

@Shopify Shopify deleted a comment from github-actions bot Oct 11, 2022
@gwyneplaine
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @gwyneplaine! 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]

@gwyneplaine
Copy link
Contributor Author

Gonna let this rock after https://github.com/Shopify/web/pull/74921 is merged.
@aaronccasanova will hold off on refactoring to your target array strategy, preference is to get this in and then iterate in a subsequent PR

@gwyneplaine
Copy link
Contributor Author

Both web PRs for this migration have now been merged:

Merging this in now, subsequent PR incoming to refactor to @aaronccasanova's collection strategy shortly 👍

@gwyneplaine gwyneplaine merged commit 8fb2158 into main Oct 19, 2022
@gwyneplaine gwyneplaine deleted the replace-sass-z-index branch October 19, 2022 02:18
@github-actions github-actions bot mentioned this pull request Oct 19, 2022
laurkim pushed a commit that referenced this pull request Oct 26, 2022
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

- [#7342](#7342)
[`dc2990acf`](dc2990a)
Thanks [@lgriffee](https://github.com/lgriffee)! - Rename
`replace-sass-length` migration to `replace-spacing-lengths`
    Add `gap` properties to `replace-sass-space` migration


- [#7310](#7310)
[`8fb215836`](8fb2158)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Add sass
z-index migration


- [#7335](#7335)
[`699b70713`](699b707)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
migration to `replace-typography-declarations`


- [#7375](#7375)
[`8194e174a`](8194e17)
Thanks [@samrose3](https://github.com/samrose3)! - Add Sass color
function migration


- [#7387](#7387)
[`a0fa96ed4`](a0fa96e)
Thanks [@lgriffee](https://github.com/lgriffee)! - Added
`replace-border-declarations` migration

### Patch Changes

- [#7487](#7487)
[`60191f353`](60191f3)
Thanks [@lgriffee](https://github.com/lgriffee)! - Update numeric
operator detection to check all node types


- [#7412](#7412)
[`6d82e8965`](6d82e89)
Thanks [@samrose3](https://github.com/samrose3)! - Enhance Sass spacing
migration to catch Sass interpolations


- [#7423](#7423)
[`0591db26a`](0591db2)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Update
`replace-typography-declarations` comment strategy


- [#7459](#7459)
[`933a9d71b`](933a9d7)
Thanks [@lgriffee](https://github.com/lgriffee)! - Add missing values to
borderRadius length map

## @shopify/[email protected]

### Minor Changes

- [#7429](#7429)
[`e87330f5b`](e87330f)
Thanks [@aveline](https://github.com/aveline)! - Update `AlphaStack` to
be polymorphic, add list reset styles and allow spacing to change based
on breakpoint


- [#7450](#7450)
[`74237ebc5`](74237eb)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`minHeight` and `minWidth` on `Box`


- [#7432](#7432)
[`85c022033`](85c0220)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`overflowX`, `overflowY`, and `width` props to `Box`


- [#7491](#7491)
[`a9eff60e1`](a9eff60)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`section` element types to `Box`


- [#7389](#7389)
[`4f184c658`](4f184c6)
Thanks [@aveline](https://github.com/aveline)! - Added `breakWord` prop
to `Text`


- [#7430](#7430)
[`87981ac01`](87981ac)
Thanks [@aveline](https://github.com/aveline)! - Added `legend` as
supported element for `Text` component


- [#7438](#7438)
[`4b4411930`](4b44119)
Thanks [@aveline](https://github.com/aveline)! - Simplify `AlphaCard` by
removing `hasBorderRadius` prop and `shadow` prop

### Patch Changes

- [#7405](#7405)
[`ea2615881`](ea26158)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed how optional `id`
prop rendered in `Text`


- [#7434](#7434)
[`62288755a`](6228875)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed reference to
`width` custom property in `Box`


- [#7396](#7396)
[`3be27bae8`](3be27ba)
Thanks [@laurkim](https://github.com/laurkim)! - Updated alias and scale
types in `Box` with type tests to check they exist in our token groups.
    Updated `get-props` script to parse utility types with unions.


- [#7435](#7435)
[`4097d539b`](4097d53)
Thanks [@kyledurand](https://github.com/kyledurand)! - Cleaned up styles
on Box


- [#7410](#7410)
[`13546e344`](13546e3)
Thanks [@kyledurand](https://github.com/kyledurand)! - Apply default
value to Columns


- [#7483](#7483)
[`7a78e07bf`](7a78e07)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed missing
`children` prop in `Columns` prop type


- [#7492](#7492)
[`612e02786`](612e027)
Thanks [@henryyi](https://github.com/henryyi)! - Only apply scroll-lock
with scrollbar when body is scrollable


- [#7481](#7481)
[`4f5f6cf50`](4f5f6cf)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed horizontal
scroll bug in Scrollable


- [#7471](#7471)
[`2f16b2310`](2f16b23)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added viewport
addon to storybook and matched values to our breakpoints


- [#7433](#7433)
[`90487fd70`](90487fd)
Thanks [@kellymiller-shop](https://github.com/kellymiller-shop)! - fixed
heading offset on IndexTable


- [#7453](#7453)
[`3191d13c0`](3191d13)
Thanks [@AndrewMusgrave](https://github.com/AndrewMusgrave)! - Fixed
IndexTable not rendering bulk actions on resize


- [#7395](#7395)
[`89fea23f7`](89fea23)
Thanks [@AndrewMusgrave](https://github.com/AndrewMusgrave)! - Fixed
scrolling performance for IndexTable on safari


- [#7442](#7442)
[`77057d5b6`](77057d5)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fix SkeletonPage
actions layout

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`dc2990acf`](dc2990a),
[`8fb215836`](8fb2158),
[`60191f353`](60191f3),
[`6d82e8965`](6d82e89),
[`0591db26a`](0591db2),
[`699b70713`](699b707),
[`8194e174a`](8194e17),
[`933a9d71b`](933a9d7),
[`a0fa96ed4`](a0fa96e)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#7396](#7396)
[`3be27bae8`](3be27ba)
Thanks [@laurkim](https://github.com/laurkim)! - Updated alias and scale
types in `Box` with type tests to check they exist in our token groups.
    Updated `get-props` script to parse utility types with unions.


- [#7393](#7393)
[`cd4577a3f`](cd4577a)
Thanks [@martenbjork](https://github.com/martenbjork)! - Update markdown
file structure to not require folders for each markdown page. Now you
can simply create a markdown.md file and it will be rendered as a page.


- [#7418](#7418)
[`c5cfe9026`](c5cfe90)
Thanks [@kyledurand](https://github.com/kyledurand)! - Support beta
status components


- [#7449](#7449)
[`31437dfb2`](31437df)
Thanks [@jjgali](https://github.com/jjgali)! - Removed duplicate image
on error messages page.


- [#7491](#7491)
[`a9eff60e1`](a9eff60)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`section` element types to `Box`


- [#7441](#7441)
[`0d446cb46`](0d446cb)
Thanks [@jjgali](https://github.com/jjgali)! - Remove section on
compositions vs patterns.


- [#7420](#7420)
[`d462e2e5d`](d462e2e)
Thanks [@jjgali](https://github.com/jjgali)! - Clarify rules for
formatting dates and times.

### Patch Changes

- [#7398](#7398)
[`c21ea83d4`](c21ea83)
Thanks [@tracytees](https://github.com/tracytees)! - - Updated the
"Getting started" and "Polaris 101" pages
- Removed /design, /develop, /shared-resources,
/patterns-components-and-tokens


- [#7426](#7426)
[`442d51ddf`](442d51d)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Fix nav bug


- [#7452](#7452)
[`8a05e840f`](8a05e84)
Thanks [@gui-santos](https://github.com/gui-santos)! - fix visual bug in
token page's navigation where text was breaking into two lines


- [#7469](#7469)
[`a8d80cfd5`](a8d80cf)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `AlphaStack`
component docs


- [#7483](#7483)
[`7a78e07bf`](7a78e07)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed missing
`children` prop in `Columns` prop type


- [#7436](#7436)
[`95a8ac0d2`](95a8ac0)
Thanks [@laurkim](https://github.com/laurkim)! - Fix references to `Box`
margin props in alpha layout examples


- [#7444](#7444)
[`e0d6b00ba`](e0d6b00)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fix tile image
and increase example widths on alpha components


- [#7427](#7427)
[`25e70a8d9`](25e70a8)
Thanks [@gwyneplaine](https://github.com/gwyneplaine)! - Declare sandbox
nav in a more idiomatic fashion


- [#7482](#7482)
[`0ec6961aa`](0ec6961)
Thanks [@alex-page](https://github.com/alex-page)! - Bump GitHub action
versions and add p-map for gen-og-images CI failure

- Updated dependencies
\[[`ea2615881`](ea26158),
[`62288755a`](6228875),
[`3be27bae8`](3be27ba),
[`4097d539b`](4097d53),
[`e87330f5b`](e87330f),
[`74237ebc5`](74237eb),
[`85c022033`](85c0220),
[`13546e344`](13546e3),
[`7a78e07bf`](7a78e07),
[`a9eff60e1`](a9eff60),
[`4f184c658`](4f184c6),
[`612e02786`](612e027),
[`4f5f6cf50`](4f5f6cf),
[`87981ac01`](87981ac),
[`2f16b2310`](2f16b23),
[`90487fd70`](90487fd),
[`3191d13c0`](3191d13),
[`89fea23f7`](89fea23),
[`77057d5b6`](77057d5),
[`4b4411930`](4b44119)]:
    -   @shopify/[email protected]

## [email protected]

### Patch Changes

- Updated dependencies
\[[`ea2615881`](ea26158),
[`62288755a`](6228875),
[`3be27bae8`](3be27ba),
[`4097d539b`](4097d53),
[`e87330f5b`](e87330f),
[`74237ebc5`](74237eb),
[`85c022033`](85c0220),
[`13546e344`](13546e3),
[`7a78e07bf`](7a78e07),
[`a9eff60e1`](a9eff60),
[`4f184c658`](4f184c6),
[`612e02786`](612e027),
[`4f5f6cf50`](4f5f6cf),
[`87981ac01`](87981ac),
[`2f16b2310`](2f16b23),
[`90487fd70`](90487fd),
[`3191d13c0`](3191d13),
[`89fea23f7`](89fea23),
[`77057d5b6`](77057d5),
[`4b4411930`](4b44119)]:
    -   @shopify/[email protected]

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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