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

Optimised appear animations #1816

Merged
merged 16 commits into from Dec 13, 2022
Merged

Optimised appear animations #1816

merged 16 commits into from Dec 13, 2022

Conversation

mattgperry
Copy link
Collaborator

@mattgperry mattgperry commented Dec 8, 2022

This PR adds the ability to play optimised appear animations via WAAPI and for Framer Motion to seamlessly take them over once React has loaded.

Animation data is shared via window and matching ids, so even if there's a hydration mismatch, the animation will continue unabated.

@mattgperry mattgperry changed the title Optimise initial animations Optimised appear animations Dec 8, 2022
@@ -56,6 +56,10 @@ const projection = Object.assign({}, config, {
format: "umd",
name: "Projection",
exports: "named",
globals: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This just removes a warning in the build script

@@ -124,7 +127,7 @@ export function animate<V = number>({
isComplete = isForwardPlayback ? state.done : elapsed <= 0
}

onUpdate?.(latest)
onUpdate && onUpdate(latest)
Copy link
Collaborator Author

@mattgperry mattgperry Dec 8, 2022

Choose a reason for hiding this comment

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

A slight aside to this PR but replacing ?. for foo && makes a surprising saving for transpiled bundle sizes.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just super-exact. onUpdate?.() becomes:

var _onUpdate;
(_onUpdate = onUpdate) === null || _onUpdate === void 0 ? void 0 : _onUpdate();

?? is also upsettingly large. const a = 0 ?? 1:

const a = (_ = 0) !== null && _ !== void 0 ? _ : 1;

@@ -109,35 +111,6 @@ export function spring({
initialDelta * Math.cos(angularFreq * t))
)
}
Copy link
Collaborator Author

@mattgperry mattgperry Dec 8, 2022

Choose a reason for hiding this comment

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

Here we're replacing the use of Motion One's spring function, which only supports under- and critically-dampened springs, with Framer Motion's which supports over-damped springs also.

To help offset the added bundlesize, we're replacing the closed-form velocity calculation with a second sample of the spring calculation. This should actually also be cheaper as the maths is simpler - not sure why we weren't already doing this.

Comment on lines +3 to +11
declare global {
interface Window {
MotionAppearAnimations?: MotionAppearAnimations
}
}
Copy link
Contributor

@nvh nvh Dec 12, 2022

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 store these on window? Let's add a comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yeah its so the inline script can communicate with Framer Motion - I've updated with a comment

Copy link
Contributor

@nvh nvh left a comment

Choose a reason for hiding this comment

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

Think we should add a comment to the reason for the window globals.

@mattgperry mattgperry force-pushed the feature/resume-waapi-animations branch from 297076c to e85e4c6 Compare December 12, 2022 16:50
@mattgperry mattgperry requested a review from nvh December 12, 2022 16:56
Copy link
Contributor

@shuangq shuangq left a comment

Choose a reason for hiding this comment

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

Code looks good 👏

Comment on lines +226 to +230
return useInstantAnimation
? valueTransition.elapsed
? () => delay(set, -valueTransition.elapsed)
: set()
: start()
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should rename the function (getAnimation) as now we changed the return type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This happens in #1818

for (const key in featureTests) {
supports[key] = () => {
if (results[key] === undefined) results[key] = featureTests[key]()
return results[key]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we return the result? 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because otherwise it wouldn't return anything and we couldn't run checks like supports.waapi()

try {
animation.cancel()
MotionAppearAnimations.delete(animationId)
} catch (e) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add some logging for the errors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add a comment, this is expected behaviour. Animation.cancel() annoyingly throws, so we have to catch it here.

Comment on lines +23 to +24
* 2. As all independent transforms share a single transform animation, stopping
* it synchronously would prevent subsequent transforms from handing off.
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't quite understand this comment, could you give an example?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in psuedo code

animate("scale", 2)
 -> handoff from "transform" animation
 -> cancel "transform" animation
animate("x", 2)
 -> no "transform" animation present, start new animation

VS

animate("scale", 2)
 -> handoff from "transform" animation
 -> schedule cancel "transform" animation
animate("x", 2)
 -> handoff from "transform" animation
 -> schedule cancel "transform" animation
cancel "transform" animation

@mattgperry mattgperry added the automerge Land this PR label Dec 13, 2022
@mergetron mergetron bot removed the autorebasing label Dec 13, 2022
@mergetron mergetron bot force-pushed the feature/resume-waapi-animations branch from 9ec28b0 to bac5a8a Compare December 13, 2022 11:48
@mergetron mergetron bot merged commit 6adc917 into main Dec 13, 2022
@mergetron mergetron bot deleted the feature/resume-waapi-animations branch December 13, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Land this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants