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-motion migration for transition easing function #7431

Closed
wants to merge 11 commits into from

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Oct 18, 2022

NOTE: Includes changes in #7403, so that PR should be merged first. In the mean-time, you can view the changes for this PR only.

WHY are these changes introduced?

ref #7213

This PR covers the transition 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):

  • 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);
    

Open Question: Do we consider cubic-bezier() / linear() / other built-in CSS easing functions as failures? I assume so (and have coded it to insert a comment when they're detected), but want to be sure before this gets merged.

@jesstelford jesstelford force-pushed the migrate-motion-easing branch from e08d5cc to 5117c24 Compare October 18, 2022 04:53
@jesstelford jesstelford marked this pull request as ready for review October 18, 2022 05:03
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 is looking 🔥 💯 love the use of the property keys in the plugin Declaration object. Is this ready to run on web or are there still edge-cases that need to be handled?

Also, can you add a small bit of documentation for this migration in the migrator package README.md?

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

@jesstelford jesstelford force-pushed the migrate-motion-easing branch from 5117c24 to e559f7d Compare October 21, 2022 02:37
@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]
yarn add @shopify/[email protected]

@jesstelford
Copy link
Contributor Author

These additions have been combined into #7403

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.

2 participants