Make `animate()` resolve for time #1993

mattgperry · 2023-03-03T14:29:27Z

This PR replaces the animate function from the previous Popmotion approach which consumes time delta per frame (helpfully illustrated by this):

Into the Motion One stateless approach where the timestamp itself is passed.

This allows for:

Playback functions. Currently we only return stop but for arbitrary seeking, pause, play of springs/inertia this stateless approach works better.
API parity with WAAPI. In a future PR this will allow animate to easily return a set of playback controls that wraps many JS and/or WAAPI animation controls.
Quicker seeking. Currently we have to simulate a low-res version of animations when sampling to ensure repeats are incorporated correctly whereas now this can be instant, which leads to faster optimised appear animation handoff and faster WAAPI interruption.

mattgperry · 2023-03-08T11:01:00Z

packages/framer-motion/package.json

@@ -103,10 +103,6 @@
            "path": "./dist/size-rollup-animate.js",
            "maxSize": "10 kB"
        },
-        {


Dupe of previous size check

packages/framer-motion/src/animation/waapi/create-accelerated-animation.ts

packages/framer-motion/src/animation/generators/utils/velocity-sample-duration.ts

packages/framer-motion/src/animation/generators/spring/index.ts

nvh · 2023-03-08T15:55:47Z

packages/framer-motion/src/animation/index.ts

-            elapsed,
+            delay: -elapsed,


I don't really get this one.

The previous animate accepted elapsed. A negative elapsed would effectively be a delay. The new animate accepts delay - a negative delay is effectively an elapsed.

That makes sense yeah, maybe we could add this as a comment, for future me who will forget about it again 😅

nvh

Left some questions, and there are still some .only( tests sprinkled here and there.

packages/framer-motion/src/animation/js/driver-frameloop.ts

packages/framer-motion/src/animation/js/index.ts

nvh

Looks good now, still have my doubts about the GroupedAnimationControl, but as it is obviously in flux I'll leave it up to you to fix now or later.

nvh · 2023-03-09T21:08:24Z

packages/framer-motion/src/animation/GroupPlaybackControls.ts

+        this.animations = animations.filter(
+            Boolean
+        ) as AnimationPlaybackControls[]


I'm still so disappointed that typescript can't figure this out 😞
Is there no way to go without the cast?

It actually didn't even work with a more explicit function either (that checked specficially for undefined)

nvh · 2023-03-09T21:09:04Z

packages/framer-motion/src/animation/GroupPlaybackControls.ts

+    get currentTime() {
+        return this.animations[0].currentTime
+    }


Shouldn't it be the max() of currentTimes or something?

That would be correct for duration, as I realised after implementing max originally too. In theory all these animations should be in sync, some could be interrupted at which point this is a bit of a fudge, we probably want to filter out animations that aren't running or paused and take the duration from the first one of those. But I do want to take a second look at currentTime before making an official API, this is just for internal use and it doesn't currently represent more than one animation, but I'll add a comment

nvh · 2023-03-09T21:10:08Z

packages/framer-motion/src/animation/animate.ts

-    return {
-        stop: () => value.stop(),
-    }
+    return value.animation || new GroupPlaybackControls([value.animation])


Why would we add the animation if it is falsy? I'd say we either always return a GroupPlaybackControls, or return an empty one when the animation doesn't exist.

nvh · 2023-03-09T21:13:11Z

packages/framer-motion/src/frameloop/index.ts

-        ? defaultTimestep
+        ? 16.666


Any reason for not making this a constant? I'd say that 1000/60 is clearer than this.

(Don't feel strongly though, as it is kinda common knowledge)

Works for me - done

mattgperry changed the base branch from main to feature/for-t March 3, 2023 14:29

mattgperry force-pushed the feature/for-t branch from b0df3b0 to e5ac1e1 Compare March 5, 2023 08:03

mattgperry force-pushed the feature/for-t-2 branch from 2a56ecb to f3fedd4 Compare March 5, 2023 08:04

mattgperry force-pushed the feature/for-t branch 3 times, most recently from 2a5c454 to 643ade9 Compare March 7, 2023 14:46

mergetron bot changed the base branch from feature/for-t to main March 7, 2023 14:49

mattgperry added 16 commits March 7, 2023 15:08

Making react optional dep

90c29c3

v10.1.0-alpha.0

6131efd

Updating lockfile

e17c806

Playing with package settings

9ae35e4

package tweaks

c590b29

Adding seperate entry point

b1922eb

v10.1.0-alpha.1

4ac392f

Adding vanilla API entrypoint

5a74a91

v10.1.0-alpha.3

c096b0f

Updating version

47d8eb2

Updating lockfile

67e14a3

Unifying playback controls types

a493bfd

Making animateValue() resolve for t

4954ac2

Adding latest resolve for t

96db3fe

Latest tests

d01a832

Fixing tests

a1813ed

mattgperry force-pushed the feature/for-t-2 branch from f3fedd4 to a1813ed Compare March 7, 2023 15:14

mattgperry added 5 commits March 7, 2023 15:40

Re-implementing currentTime

835dbdf

Fixing tests

58d79dd

Removing currentTime

4fd5ee4

Removing deps

5e4a7e0

Fixing test script

a4e584d

mattgperry commented Mar 8, 2023

View reviewed changes

nvh reviewed Mar 8, 2023

View reviewed changes

packages/framer-motion/src/animation/waapi/create-accelerated-animation.ts Outdated Show resolved Hide resolved

nvh reviewed Mar 8, 2023

View reviewed changes

packages/framer-motion/src/animation/generators/utils/velocity-sample-duration.ts Outdated Show resolved Hide resolved

nvh reviewed Mar 8, 2023

View reviewed changes

packages/framer-motion/src/animation/generators/spring/index.ts Outdated Show resolved Hide resolved

nvh reviewed Mar 8, 2023

View reviewed changes

nvh requested changes Mar 8, 2023

View reviewed changes

packages/framer-motion/src/animation/js/driver-frameloop.ts Show resolved Hide resolved

packages/framer-motion/src/animation/js/index.ts Outdated Show resolved Hide resolved

mattgperry added 12 commits March 8, 2023 16:10

Removing only

eb39e7b

Removing unused type

e0d8d4c

Removing only

47554ca

Removing todo

def6795

Removing extra file

9f90a63

Removing zero func

22403e4

Removing unused type

fd7d3f7

Fixing timestep

23cfddd

Implementing GroupAnimationControls

c926c95

Latest

bbe37b6

Ensuring using time conversion utility functions

5809dbf

Making maxDuration labelled constant

be1f608

mattgperry requested a review from nvh March 9, 2023 11:49

mattgperry added 4 commits March 9, 2023 13:23

Flaky test

49efac8

Fixing flaky test

0a23bff

Fixing test

edf196a

Flaky test

f36c819

nvh approved these changes Mar 9, 2023

View reviewed changes

mattgperry added 2 commits March 10, 2023 09:49

Adding comment

1b18a7e

Removing falsey animation

d44960a

mattgperry added the automerge Land this PR label Mar 10, 2023

Flaky test:

19b967a

mergetron bot merged commit f253ab6 into main Mar 10, 2023

mergetron bot deleted the feature/for-t-2 branch March 10, 2023 10:43

chuganzy mentioned this pull request Jul 28, 2023

Fix incorrect Reorder.Item order calculation #2231

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `animate()` resolve for time #1993

Make `animate()` resolve for time #1993

mattgperry commented Mar 3, 2023 •

edited

mattgperry Mar 8, 2023

nvh Mar 8, 2023

mattgperry Mar 8, 2023 •

edited

nvh Mar 9, 2023

nvh left a comment

nvh left a comment

nvh Mar 9, 2023

mattgperry Mar 10, 2023

nvh Mar 9, 2023

mattgperry Mar 10, 2023

nvh Mar 9, 2023

nvh Mar 9, 2023

nvh Mar 9, 2023

mattgperry Mar 10, 2023

Make animate() resolve for time #1993

Make animate() resolve for time #1993

Conversation

mattgperry commented Mar 3, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattgperry Mar 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvh left a comment

Choose a reason for hiding this comment

nvh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Make `animate()` resolve for time #1993

Make `animate()` resolve for time #1993

mattgperry commented Mar 3, 2023 •

edited

mattgperry Mar 8, 2023 •

edited