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

[Migrator] Add migrate-sass-transition migration for transition durations & easing #7403

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Oct 14, 2022

NOTE: This leverages the functionality in #7543 to get some sweet sweet clean code and juicy verbose output!

WHY are these changes introduced?

ref #7213

This PR covers the transition, transition-duration, transition-delay, and transition-timing-function properties, their various complex uses and a couple of edge cases as seen in the wild.

WHAT is this pull request doing?

Example migrations (taken from the test files):

duration

  • transition-duration: 100ms;
    transition-duration: var(--p-duration-100);
  • transition-duration: legacy-polaris-v8.duration('slow');
    transition-duration: var(--p-duration-300);
  • transition: opacity 100ms linear;
    transition: opacity var(--p-duration-100) linear;
  • transition: opacity legacy-polaris-v8.duration('slow') linear;
    transition: opacity var(--p-duration-300) linear;
  • transition: opacity 100ms linear, left 100ms linear;
    transition: opacity var(--p-duration-100) linear, left var(--p-duration-100) linear;
  • transition: opacity legacy-polaris-v8.duration(slow) linear, left legacy-polaris-v8.duration(slow) linear;
    transition: opacity var(--p-duration-300) linear, left var(--p-duration-300) linear;
  • transition: opacity legacy-polaris-v8.duration(slower) linear legacy-polaris-v8.duration(fast);
    transition: opacity var(--p-duration-400) linear var(--p-duration-100);

easing

  • transition-timing-function: linear
    transition-timing-function: var(--p-linear)
  • transition-timing-function: legacy-polaris-v8.easing('in')
    transition-timing-function: var(--p-ease-in);
  • transition: opacity 300ms legacy-polaris-v8.easing('base');
    transition: opacity var(--p-duration-300) var(--p-ease);

Will output various error messages for things it can't migrate:

  • transition-timing-function: legacy-polaris-v8.easing(anticipate);
    /* polaris-migrator: Unable to migrate the following expression. Please upgrade manually. The anticipate easing function is no longer available in Polaris. See https://polaris.shopify.com/tokens/motion for possible values. */
    transition-timing-function: legacy-polaris-v8.easing('anticipate');
    
  • transition-timing-function: cubic-bezier(0, 0, 1, 1);
    /* polaris-migrator: Unable to migrate the following expression. Please upgrade manually. Unexpected easing function 'cubic-bezier'. See https://polaris.shopify.com/tokens/motion for possible values. */
    transition-timing-function: cubic-bezier(0, 0, 1, 1);
    

Notes

  1. We consider cubic-bezier() / linear() / other built-in CSS easing functions as failures (inserting a comment when they're detected).
  2. We consider use of variables ($foo) as failures (because we can't statically analyse what their value is).

Copy link
Contributor

@gwyneplaine gwyneplaine left a comment

Choose a reason for hiding this comment

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

lgtm

@gwyneplaine
Copy link
Contributor

gwyneplaine commented Oct 20, 2022

/stageit
woops wrong command

@github-actions
Copy link
Contributor

🚀 9e808f3ed30f0d184084af0a03ad89784f26c098 is now deploying to staging. View status.

@gwyneplaine
Copy link
Contributor

/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]
yarn add @shopify/[email protected]

@sam-b-rose sam-b-rose mentioned this pull request Oct 20, 2022
19 tasks
@jesstelford jesstelford force-pushed the migrate-motion branch 2 times, most recently from 7e482b0 to ecfa5b5 Compare October 26, 2022 02:49
@jesstelford jesstelford changed the title [Migrator] Add migrate-motion migration for transition durations [Migrator] Add migrate-sass-transition migration for transition durations & easing Oct 26, 2022
@jesstelford jesstelford force-pushed the migrate-motion branch 2 times, most recently from ff3f5c9 to 5d669f5 Compare October 28, 2022 06:18
@jesstelford jesstelford changed the base branch from main to stylelint-partial-fix-reporting October 28, 2022 06:18
@jesstelford
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

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

@jesstelford
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

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

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.

Very thorough! I say we keep this migration moving and motion forward 🚀

We will likely want to split out the Sass function replacement with the tokenization of declaration values, but for now this is a safe start so we can start migrating in Shopify/web.

export function isPolarisVar(node: Node): boolean {
return (
isSassFunction('var', node) &&
(node.nodes?.[0]?.value ?? '').startsWith('--p-')
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can be extra sure this is a Polaris token by leveraging this RegEx logic here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, however that would mean we need to generate a gigantic regex that contains all polaris tokens (since we're not only checking for colors as in that linked example).

Is there a reason the prefix check wouldn't work? Perhaps it returns false positives?

Copy link
Member

@sam-b-rose sam-b-rose Nov 2, 2022

Choose a reason for hiding this comment

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

Just added safety is all. More concerned around other 3P apps using the migration and possibly having custom properties prefixed with --p- as well. Low likelihood but possible 🤷 I'm happy with your current logic as well. My comment was more of a suggestion for added confidence that we are replacing the right tokens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point: I forgot about 3P devs! I've made that change 👍

@jesstelford jesstelford force-pushed the stylelint-partial-fix-reporting branch from 9727216 to 9f28137 Compare November 2, 2022 23:06
Base automatically changed from stylelint-partial-fix-reporting to main November 2, 2022 23:06
@jesstelford jesstelford force-pushed the migrate-motion branch 3 times, most recently from e4ea442 to b5d8db5 Compare November 2, 2022 23:30
@jesstelford jesstelford merged commit 8859f5d into main Nov 3, 2022
@jesstelford jesstelford deleted the migrate-motion branch November 3, 2022 01:00
@github-actions github-actions bot mentioned this pull request Nov 3, 2022
laurkim pushed a commit that referenced this pull request Nov 4, 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

- [#7548](#7548)
[`432bdd5fe`](432bdd5)
Thanks [@anthonymenecola](https://github.com/anthonymenecola)! - add
cancel major icon


- [#7620](#7620)
[`35be8a003`](35be8a0)
Thanks [@rdott](https://github.com/rdott)! - Added inactive location
minor and major icons

## @shopify/[email protected]

### Minor Changes

- [#7403](#7403)
[`8859f5db5`](8859f5d)
Thanks [@jesstelford](https://github.com/jesstelford)! - Introduce
`migrate-motion` migration for migrating `transition`,
`transition-duration`, and `transition-delay` usages of duration values.


- [#7606](#7606)
[`cf7badbd1`](cf7badb)
Thanks [@samrose3](https://github.com/samrose3)! - Renamed and split
migrations based on scope and type (react, scss, and styles)


- [#7543](#7543)
[`8c1989618`](8c19896)
Thanks [@jesstelford](https://github.com/jesstelford)! - Expose
utilities for SASS Migrations to leverage the Suggestion-on-partial-fix
pattern


- [#7529](#7529)
[`3652eb901`](3652eb9)
Thanks [@samrose3](https://github.com/samrose3)! - Add relative option
for replace-text-component migration


- [#7532](#7532)
[`ba576498d`](ba57649)
Thanks [@jesstelford](https://github.com/jesstelford)! - Expose the
.report() method to SASS migrations for easier aggregation of discovered
issues during a migration run.

### Patch Changes

- [#7606](#7606)
[`cf7badbd1`](cf7badb)
Thanks [@samrose3](https://github.com/samrose3)! - Update
`createInlineComment` to format text with RegExp


- [#7606](#7606)
[`cf7badbd1`](cf7badb)
Thanks [@samrose3](https://github.com/samrose3)! - Add support to
replace Identifiers along with JSXIdentifiers for Text migration


- [#7632](#7632)
[`1f2ec8bfe`](1f2ec8b)
Thanks [@samrose3](https://github.com/samrose3)! - Check for targeted
component import before modifying in Text component migration

- Updated dependencies
\[[`6e9edd3b5`](6e9edd3)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#7572](#7572)
[`20c8cad81`](20c8cad)
Thanks [@laurkim](https://github.com/laurkim)! - Replaced usage of text
components in component stories with `Text` component


- [#7621](#7621)
[`6e9edd3b5`](6e9edd3)
Thanks [@aveline](https://github.com/aveline)! - - Added border width
prop to `Box`
- Exported color token subset alias types from tokens package and remove
from `Box`


- [#7068](#7068)
[`ccdcea22e`](ccdcea2)
Thanks [@laurkim](https://github.com/laurkim)! - Deprecated
`DisplayText`, `Heading`, `Subheading`, `Caption`, `TextStyle`, and
`VisuallyHidden` components

### Patch Changes

- [#7644](#7644)
[`b3e73ee04`](b3e73ee)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added horizontal
spacing defaults to `Bleed`


- [#7530](#7530)
[`79d92a820`](79d92a8)
Thanks [@samrose3](https://github.com/samrose3)! - Replaced all
typography components with the new `Text` component.
    Added support for `text-inverse` color type on `Text`.
Removed references to the following mixins to use the new `Text` or
tokens directly in classes: `text-style-body`, `text-style-heading`,
`text-style-subheading`, `text-style-caption`, `text-style-button`,
`text-style-button-large`, `text-emphasis-subdued`,
`text-emphasis-strong`, `nav-item-text-attributes`.


- [#7577](#7577)
[`db951f855`](db951f8)
Thanks [@RickyMarou](https://github.com/RickyMarou)! - Page component:
display subtitle even when it's the only header prop set


- [#7633](#7633)
[`1364be7f1`](1364be7)
Thanks [@kyledurand](https://github.com/kyledurand)! - Renamed `alignY`
prop to `alignBlock` on `Inline`
    Added more flex properties to `align` on `Inline`


- [#7443](#7443)
[`7a6fb7c1c`](7a6fb7c)
Thanks [@iAmNathanJ](https://github.com/iAmNathanJ)! - Improve
performance of the Scrollable component with React 18


- [#7625](#7625)
[`9f8b651dd`](9f8b651)
Thanks [@kyledurand](https://github.com/kyledurand)! - Removed wrap
children with div from Inline component


- [#7593](#7593)
[`addd6bcdd`](addd6bc)
Thanks [@kyledurand](https://github.com/kyledurand)! - Improved comments
across layout components, changed default spacing of Inline component to
match AlphaStack


- [#7600](#7600)
[`f006509be`](f006509)
Thanks [@billycai](https://github.com/billycai)! - Add spacing between
title and metadata for Page component


- [#7563](#7563)
[`a9051d678`](a9051d6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Inline`
component docs and default prop values


- [#7635](#7635)
[`3cb5377a6`](3cb5377)
Thanks [@iAmNathanJ](https://github.com/iAmNathanJ)! - Fixed Scrollable
component to match existing onScrolledToBottom logic

- Updated dependencies
\[[`432bdd5fe`](432bdd5),
[`6e9edd3b5`](6e9edd3),
[`35be8a003`](35be8a0)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#7621](#7621)
[`6e9edd3b5`](6e9edd3)
Thanks [@aveline](https://github.com/aveline)! - - Added border width
prop to `Box`
- Exported color token subset alias types from tokens package and remove
from `Box`

## @shopify/[email protected]

### Minor Changes

- [#7551](#7551)
[`d7dc4436f`](d7dc443)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
`stylelint-polaris/coverage` rule

### Patch Changes

- [#7589](#7589)
[`b7b0ef5a9`](b7b0ef5)
Thanks [@aaronccasanova](https://github.com/aaronccasanova)! - Add
constraints to `stylelint-polaris/coverage` disable comments

- Updated dependencies
\[[`6e9edd3b5`](6e9edd3)]:
    -   @shopify/[email protected]

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`8859f5db5`](8859f5d),
[`cf7badbd1`](cf7badb),
[`cf7badbd1`](cf7badb),
[`cf7badbd1`](cf7badb),
[`8c1989618`](8c19896),
[`3652eb901`](3652eb9),
[`1f2ec8bfe`](1f2ec8b),
[`ba576498d`](ba57649)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#7068](#7068)
[`ccdcea22e`](ccdcea2)
Thanks [@laurkim](https://github.com/laurkim)! - Deprecated
`DisplayText`, `Heading`, `Subheading`, `Caption`, `TextStyle`, and
`VisuallyHidden` pages and removed examples


- [#7609](#7609)
[`343865159`](3438651)
Thanks [@sarahill](https://github.com/sarahill)! - Added new type style
guidance and info to typography docs

### Patch Changes

- [#7634](#7634)
[`4db441756`](4db4417)
Thanks [@laurkim](https://github.com/laurkim)! - Replaced usage of
typography components (`DisplayText`, `Heading`, `Subheading`,
`Caption`, `VisuallyHidden`, `TextStyle`) with the new `Text` component


- [#7604](#7604)
[`aa82c82ff`](aa82c82)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Inline`
component doc vertical alignment example


- [#7568](#7568)
[`ab0cf251f`](ab0cf25)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `AlphaCard`
component guidance and examples


- [#7633](#7633)
[`1364be7f1`](1364be7)
Thanks [@kyledurand](https://github.com/kyledurand)! - Renamed `alignY`
prop to `alignBlock` on `Inline`
    Added more flex properties to `align` on `Inline`


- [#7527](#7527)
[`924e9e5cd`](924e9e5)
Thanks [@chazdean](https://github.com/chazdean)! - Update `Columns`
component docs


- [#7596](#7596)
[`749ee31ee`](749ee31)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed home promo
image layout


- [#7563](#7563)
[`a9051d678`](a9051d6)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Inline`
component docs and default prop values


- [#7566](#7566)
[`567822218`](5678222)
Thanks [@kyledurand](https://github.com/kyledurand)! - Bumped nextjs


- [#7602](#7602)
[`9931ce0b4`](9931ce0)
Thanks [@kyledurand](https://github.com/kyledurand)! - Bumped nextjs to
13.0.1


- [#7571](#7571)
[`4c5ccc8fa`](4c5ccc8)
Thanks [@chazdean](https://github.com/chazdean)! - Updated `Bleed`
component guidance and examples

- Updated dependencies
\[[`b3e73ee04`](b3e73ee),
[`20c8cad81`](20c8cad),
[`79d92a820`](79d92a8),
[`db951f855`](db951f8),
[`432bdd5fe`](432bdd5),
[`6e9edd3b5`](6e9edd3),
[`ccdcea22e`](ccdcea2),
[`1364be7f1`](1364be7),
[`7a6fb7c1c`](7a6fb7c),
[`9f8b651dd`](9f8b651),
[`addd6bcdd`](addd6bc),
[`f006509be`](f006509),
[`a9051d678`](a9051d6),
[`3cb5377a6`](3cb5377),
[`35be8a003`](35be8a0)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]
    -   @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