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

docs(motion-presets): Motion Refresh / Presets RFC 2024 #2336

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

saurabhdaware
Copy link
Member

@saurabhdaware saurabhdaware commented Aug 22, 2024

This comment was marked as off-topic.

Copy link
Contributor

github-actions bot commented Aug 22, 2024

✅ PR title follows Conventional Commits specification.

Copy link

codesandbox-ci bot commented Aug 22, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0419b6c:

Sandbox Source
razorpay/blade: basic Configuration

@rzpcibot
Copy link
Collaborator

rzpcibot commented Aug 22, 2024

Bundle Size Report

No bundle size changes detected.

Generated by 🚫 dangerJS against 0419b6c


## Library Comparison

### Goals of Ideal Library
Copy link
Collaborator

Choose a reason for hiding this comment

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

Our goals should also tie in with design goals... evaluate libraries to see if something we want to do on design whether or not we can do it on dev. Good example you've added is morph animations... do we want to do morph now or later as per design?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup I've added morph and page transitions based on design expectations. We're doing morph now only.

Other things that are on design are simple opacity, slide, fade, etc transitions which are possible in every library

| **Easy to implement complex animations** | ✅ (Declarative API) | ❌ | ❌ | ❌ |
| **React Router Page Transition** | ✅ | ❌ (No native support but can be implemented) | ✅ | ❌ (No native support but can be implemented) |
| **Morph Animations / Layout Animations** | ✅ [Framer Motion POC](#morph-animations-with-framer-motion) | ❌ | ✅ | ❌ |
| **Small bundle size** | ❌ (4.6kb core + 40kb features that can be lazy loaded) | ✅ (4kb ) | ❌ (26kb core + features) | ✅ (0kb) |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets detail out the bundle size for framer motion. How will we load the core module only? In which cases will we need rest of the bundles?


### Morph / Layout Animations with Framer Motion

https://github.com/user-attachments/assets/ad3d4a23-c3b9-4980-a051-a0f44e7224dc
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try doing the morph animation with blade components?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup will do that POC with API decision now

Copy link
Member

Choose a reason for hiding this comment

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

We should try to replicate the Switch animation and see how our primitives plays out.

Switch has a 2 stage animation

Click Hold (Elongate the circle) -> Release (De-elongate the circle + move to the opposite side)

Scale, transformX both will have to animate at once.

| ---------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------- | -------------------------------------- | --------------------------------------------- |
| **License** (Preferrably free to use) | ✅ MIT | ✅ MIT | ❌ (Commercial License + Paid Plugins) | ✅ No License |
| **Hardware Accelarated Animations** | ✅ (Hybrid - WAAPI for some transformations with fallback to JS) [Hardware Accelarated POC](#hardware-accelarated-motion-using-framer-motion) | ✅ (Built on WAAPI) | ❌ | ✅ |
| **Easy to implement complex animations** | ✅ (Declarative API) | ❌ | ❌ | ❌ |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Motion one seems super light-weight. What is the difficulty in implementing animations with it? Can you add an example of an animation with framer vs with motion one?

Since we will be exposing presets to our end consumers won't it make sense to use a light-weight library within blade and abstract out complex animations within the presets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will add more details here. Majority of things come from this article https://motion.dev/blog/should-i-use-framer-motion-or-motion-one (Find "Design goals" heading)

Overall its a low-level imperative API so requires us to call animate() function in vanilla JS and write react wrappers on top of it (which is something that we can do with CSS as well)

It lacks support for some important features like layout animations, page transitions, which we're expecting to do. So that will require custom implementation from scratch. It would've worked if we only had opacity, fade, transform transitions but we're doing more than that . If we implement such things from scratch, we'll anyway reach closer to framer-motion minified bundle.

There are projects like modular onboarding, frontend-website which are already using framer-motion and we have't seen load-time issues there. frontend-website had runtime laggy animation issues which are solved in their new update.

@saurabhdaware saurabhdaware changed the title rfc(motion-presets): Motion Refresh / Presets RFC 2024 docs(motion-presets): Motion Refresh / Presets RFC 2024 Aug 26, 2024
@saurabhdaware saurabhdaware marked this pull request as ready for review September 9, 2024 06:05
3. Easy to implement complex animations
4. React Router page transition support
5. Morph Animations / Layout Animations
6. Small bundle-size
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we add Cross Platform as a goal as well? even though we might implement it later but we need to evaluate for RN as well. Eg: Vanilla CSS might not work on RN and we might as well use a diff lib for RN like ReAnimated

Copy link
Member Author

@saurabhdaware saurabhdaware Sep 13, 2024

Choose a reason for hiding this comment

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

I think I can add as a note for react native here. Not sure if it'll make sense to add it as a factor in the library comparison table because no web animation library really supports react native so the library exploration for web and native will be different.

I can add a note saying how we use react-native-reanimated already and some of these presets can be built there. I saw that react-native-reanimated already supports a good amount of these presets. So we can re-export some of these with similar API in future - https://docs.swmansion.com/react-native-reanimated/docs/layout-animations/entering-exiting-animations

>
> The API decisions here are only there to give some basic idea on the structure and usage. More accurate props will be updated here later once they are finalised in design.

### Entry / Exit Animation Presets
Copy link
Collaborator

Choose a reason for hiding this comment

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

So I was wondering can we go more specific here and in presets instead of giving like Fade, Scale presets what if we form patterns like PageTransition, Toggle, etc.

pros: this way it will reduce the learning curve and confusion and also minimise random animations for similar things and move us towards our eventual goal of streamlining patterns

cons: We might have to export similar behaviors like Fade, Scale in different components, might be reptitive. This might also feel restrictive at first

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes I was of similar opinion and I discussed this with design but what we realised is its valid case where card on some page might move up whereas card in some other page might fade in. We can't at one place define how section or component appears without knowing context.

From tech perspective I was also thinking of API like <Card animate> but in that case framer motion will always end up in bundle irrespective of whether animations are used or not since we'll have to define the node as motion node.

What I was also thinking is, once we have overall idea of animation patterns. Similar to how we have components and how we'll have patterns, we can have motion components and motion patterns (or incorporate motion in some our component patterns itself)

Copy link
Member Author

Choose a reason for hiding this comment

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

I added our discussion points and why we're going this way in open questions section along with conclusion


# Adoption strategy

- We plan to target 1 project this quarter (Q2) to get motion adopted
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- We plan to target 1 project this quarter (Q2) to get motion adopted
- We plan to target 1 project this quarter (Q3) to get motion adopted

??

- The new projects that are built, should be built with motion presets on design and dev
- The earlier project that we have should use motion presets when they redesign / revamp

# How do we educate people?
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets add how will we get this implemented on Figma for easing the hand-off as well here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup I'll update it here. We're still figuring out the right approach. We discussed few things yesterday, RK will just confirm it once if things are working fine or not


</details>

#### AnimateInteractions
Copy link
Member

Choose a reason for hiding this comment

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

What does AnimateInteractions do internally? Can you provide an overview?

Copy link
Member Author

Choose a reason for hiding this comment

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

It creates motion.div with whileHover, whileTap, whileFocus etc (depending on which interactions you want. Taking it as a prop), it defines the name of the variant of its child.

You can check this POC - https://codesandbox.io/p/sandbox/framer-motion-hover-presets-poc-ghlcpl

The Card component in this example is basically the AnimateInteractions

Comment on lines 314 to 319
<AnimateInteractions>
<Card>
<CardBody>
<Scale>
<img src="./rajorpay.jpeg" />
</Scale>
Copy link
Member

Choose a reason for hiding this comment

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

This feels a bit "Magic" with AnimateInteraction automatically making the Scale working.

What if I want the Scale to trigger based upon some other state or timer?

Eg: Scale an icon button to make attract attention after a set duration.

Copy link
Member

Choose a reason for hiding this comment

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

And in here what is AnimateInteraction doing? It will create another wrapper with hover events attached?

Define behaviour of:

  1. What happens on focusing the AnimateInteractions? should it have focus?
  2. How will it work in conjunction with shouldScaleOnHover prop of Card
  3. AnimateInteractions can be inside Card Body?

Copy link
Member Author

Choose a reason for hiding this comment

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

This feels a bit "Magic" with AnimateInteraction automatically making the Scale working.

True true. I didn't know how the props would be defined while writing this. I feel this feels less magic (this is the final code after implementation) -

Scale when parent is hovered

<AnimateInteraction motionTriggers={['hover']}>
  <SomeOtherCompoennt>
    <Scale><ComponentToScale /></Scale>
  </SomeOtherComponents>
</AnimateInteraction>

Scale when parent is tapped

<AnimateInteraction motionTriggers={['tap']}>
  <SomeOtherCompoennt>
    <Scale><ComponentToScale /></Scale>
  </SomeOtherComponents>
</AnimateInteraction>

Scale when the component itself is hovered

<Scale motionTriggers={['hover']}><ComponentToScale /></Scale>

Comment on lines +352 to +360
<Fade>
<Box />
</Fade>
<Fade>
<Box />
</Fade>
<Fade>
<Box />
</Fade>
Copy link
Member

Choose a reason for hiding this comment

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

What if I mix Scale, Move etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

works

```jsx
import { Fade, Card, CardBody } from '@razorpay/components/blade';

<Fade isVisible={showCard}>
Copy link
Member

Choose a reason for hiding this comment

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

How do we pass duration tokens etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

we can't. Which is why we have preset so that we can define durations and easing on preset level and make it consistent

*
* @default ['mount']
*/
motionTriggers: ('mount' | 'hover' | 'tap' | 'inView')[];
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to define tap and inview triggers?

Consumers can do this on their end also like so:

const isInView = useIsInView()

<Fade isVisible={isInView} />

Also if we are supporting tap, then we need to support click handlers too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added all the triggers here which are natively supported by framer-motion so API is simpler

https://www.framer.com/motion/scroll-animations/#scroll-triggered-animations

Also if we are supporting tap, then we need to support click handlers too?

Nope. Click handlers can be added to children directly no if they support it? This is purely for animation component. It shouldn't be used to add any logic.

E.g. I can just add click handlers on children if needed for some logic

<Fade>
  <Card onClick={() => } />
</Fade>

Comment on lines +299 to +301
<AnimateInteractions motionTriggers={['hover']}>
<Card>
<CardBody>
Copy link
Member

Choose a reason for hiding this comment

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

Will AnimateInteraction create a new div wrapper? or is this a enhancer component?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to create a div wrapper because something was not working. Lemme check again and see if it can be enhancer component

```

```ts
type FadeProps = {
Copy link
Member

Choose a reason for hiding this comment

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

Should we provide atleast the delay prop to every primitive?

So that consumers can choose to delay the animation run by x duration?

Copy link
Member Author

Choose a reason for hiding this comment

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

These props are not finalized. Only structure is. Will add props depending on discussion with design

Choose a reason for hiding this comment

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

On the same note, should we have tokens for duration if we don't allow explicit values? Certain use cases might need animations to run for different times. I understand it won't be an issue for looped ones, but for non-looped ones, this will help

Copy link
Member Author

Choose a reason for hiding this comment

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

Atleast from design-side we're saying that the enter and exit durations should have same duration always. Do you know any usecase where a single duration won't work? we can also add this prop in future if usecase comes since this anyways won't be a breaking change then.


// This animation will run when loadFeatures resolves.
function App({ children }) {
return <LazyMotion features={loadFeatures}>{children}</LazyMotion>;
Copy link
Member

Choose a reason for hiding this comment

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

Will consumer do this or we do this?

We will anyways have to use the framer-motion's provider right because blade components itself may also use the motion primitives.

But if we already have the provider and as a library we can't really do lazy import on our end then we don't really have choice other than to use the full framer motion right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Consumer will do it. It will be required peer dependency.

We will use it in our components. Sort of like i18nify.

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.

6 participants