feat(motion): Refactor scale, migrate to new variant structure #33341
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
packages/react-components/react-motion-components-preview/library/src/components/Scale/Scale.ts
Show resolved
Hide resolved
| export const createScalePresence: PresenceMotionFnCreator<ScaleVariantParams_unstable, ScaleRuntimeParams_unstable> = | ||
| ({ | ||
| enterDuration = motionTokens.durationGentle, | ||
| enterEasing = motionTokens.curveEasyEaseMax, |
There was a problem hiding this comment.
| enterEasing = motionTokens.curveEasyEaseMax, | |
| enterEasing = motionTokens.curveDecelerateMax, |
Somehow this value was changed in the diff, but it hasn't changed in the spec. The Scale variants all use decelerate max for enter and accelerate max for exit.
| @@ -0,0 +1,20 @@ | |||
| // eslint-disable-next-line @typescript-eslint/naming-convention | |||
| export type ScaleVariantParams_unstable = { | |||
There was a problem hiding this comment.
Good choice to mark these as unstable in the naming convention, since there is more design work that needs to be done.
| scaleAtom({ | ||
| duration: exitDuration, | ||
| easing: exitEasing, | ||
| }), |
There was a problem hiding this comment.
It looks like the exit scale atom is exactly the same as the enter, rather than scaling in reverse.
| easing: string; | ||
| }): AtomMotion => ({ | ||
| keyframes: [ | ||
| { transform: `scale3d(${fromScale}, ${fromScale}, 1)`, visibility: 'visible' }, |
There was a problem hiding this comment.
The previous implementation used both visibility of hidden and visible between enter and exit transitions. I haven't tested this new code yet, but I wonder if it functions as expected.
| easing: exitEasing, | ||
| }), | ||
| ] | ||
| : []), |
There was a problem hiding this comment.
I understand the thinking behind this refactoring and the functional style, and the motivations are fine. But with the resulting code, it's debatable as to whether the readability has become better or worse. The ternary spread is clever and functional, but it's verbose and not straightforward to understand like a simple imperative if.
|
There are 2 independent changes happening in this PR:
The code for 1) is good, and it's what is described in the ticket. |
|
Also, note that the previous migration for
This needs to be done for Scale's Storybook example as well: |
Previous Behavior
New Behavior
Scalecomponent to adopt a newcreateScalePresencefunction for defining motion parameters.ScaleVariantParams_unstableandScaleRuntimeParams_unstableto enhance type safety and flexibility.ScaleSnappyandScaleRelaxedcomponents using the refactored variant structure.Key Changes:
Refactored
Scale.tsto utilize the new createScalePresence approach.Created new reusable atoms (
scaleAtom,opacityAtom) for motion logic.Defined new TypeScript types for scale variant parameters.
Simplified and improved motion logic with better defaults.
Fixes #33112