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.

Sign up for GitHub

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

Ensure optimised appear animations are cancelled when styles differ #2772

Merged
merged 8 commits into from
Aug 29, 2024
Merged

Ensure optimised appear animations are cancelled when styles differ #2772

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
df3fe5d
Ensure values cancelled on scroll
mattgperry Aug 23, 2024
66a0c0e
Fixing
mattgperry Aug 23, 2024
12a03d3
Latest
mattgperry Aug 23, 2024
8959682
Updating
mattgperry Aug 23, 2024
aef7afc
Updating appear tests json
mattgperry Aug 23, 2024
1a0fa2a
Remove logging
mattgperry Aug 23, 2024
3ea535f
Cleaning PR
mattgperry Aug 29, 2024
fb7677b
Adding tesst
mattgperry Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions dev/html/public/optimized-appear/defer-handoff-external-values.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<html>
<head>
<style>
body {
padding: 100px;
margin: 0;
}

#box {
width: 100px;
height: 100px;
background-color: #0077ff;
}

[data-layout-correct="false"] {
background: #dd1144 !important;
opacity: 1 !important;
}
</style>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/imports/optimized-appear.js"></script>
<script type="module" src="/src/imports/script-assert.js"></script>

<script type="module">
const {
motion,
animateStyle,
animate,
startOptimizedAppearAnimation,
optimizedAppearDataAttribute,
motionValue,
frame,
} = window.Motion
const { matchViewportBox } = window.Assert
const root = document.getElementById("root")

const duration = 0.5
const x = motionValue(0)
const animateX = motionValue(0)

let isFirstFrame = true

function Component() {
React.useEffect(() => {
setTimeout(() => {
x.set(200)
}, 200)
}, [])

return React.createElement(motion.div, {
id: "box",
initial: { x: 0, opacity: 0 },
animate: { x: 100, opacity: 1 },
transition: {
duration,
ease: "linear",
layout: { ease: () => 0, duration: 10 },
},
style: {
x,
position: "relative",
background: "blue",
},
values: { x: animateX },
onAnimationComplete: () => {
const box = document.getElementById("box")
const { left } = box.getBoundingClientRect()

if (Math.round(left) !== 300) {
showError(
box,
`optimised animation not cancelled by external value mismatch with rendered style`
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The opacity animation should not be canceled, right? Do we also want to check that as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I'll add a check - it should be cancelled by mismatches as well

},
[optimizedAppearDataAttribute]: "a",
children: "Content",
})
}

// Emulate server rendering of element
root.innerHTML = ReactDOMServer.renderToString(
React.createElement(Component)
)

// Start optimised opacity animation
startOptimizedAppearAnimation(
document.getElementById("box"),
"opacity",
[0, 1],
{
duration: duration * 1000,
ease: "linear",
}
)

// Start WAAPI animation
const animation = startOptimizedAppearAnimation(
document.getElementById("box"),
"transform",
["translateX(0px)", "translateX(100px)"],
{
duration: duration * 1000,
ease: "linear",
},
(animation) => {
setTimeout(() => {
ReactDOM.hydrateRoot(
root,
React.createElement(Component)
)
}, (duration * 1000) / 4)
}
)
</script>
</body>
</html>
2 changes: 1 addition & 1 deletion packages/framer-motion/cypress/fixtures/appear-tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
["defer-handoff-block.html","defer-handoff-layout-ancestor-suspend.html","defer-handoff-layout-ancestor.html","defer-handoff-layout-child.html","defer-handoff-layout-opacity.html","defer-handoff-layout-sibling-transform.html","defer-handoff-layout-sibling.html","defer-handoff-layout-useeffect.html","defer-handoff-layout-uselayouteffect.html","defer-handoff-layout.html","defer-handoff.html","interrupt-delay-after.html","interrupt-delay-before-accelerated.html","interrupt-delay-before.html","interrupt-spring.html","interrupt-tween-opacity-waapi.html","interrupt-tween-opacity.html","interrupt-tween-transforms.html","interrupt-tween-x.html","persist-optimised-animation.html","persist.html","portal.html","resync-delay.html","resync.html","start-after-hydration.html"]
["defer-handoff-block.html","defer-handoff-external-values.html","defer-handoff-layout-ancestor-suspend.html","defer-handoff-layout-ancestor.html","defer-handoff-layout-child.html","defer-handoff-layout-opacity.html","defer-handoff-layout-sibling-transform.html","defer-handoff-layout-sibling.html","defer-handoff-layout-useeffect.html","defer-handoff-layout-uselayouteffect.html","defer-handoff-layout.html","defer-handoff.html","interrupt-delay-after.html","interrupt-delay-before-accelerated.html","interrupt-delay-before.html","interrupt-spring.html","interrupt-tween-opacity-waapi.html","interrupt-tween-opacity.html","interrupt-tween-transforms.html","interrupt-tween-x.html","persist-optimised-animation.html","persist.html","portal.html","resync-delay.html","resync.html","start-after-hydration.html"]
4 changes: 2 additions & 2 deletions packages/framer-motion/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"bundlesize": [
{
"path": "./dist/size-rollup-motion.js",
"maxSize": "33.92 kB"
"maxSize": "34.05 kB"
},
{
"path": "./dist/size-rollup-m.js",
Expand All @@ -97,7 +97,7 @@
},
{
"path": "./dist/size-rollup-dom-max.js",
"maxSize": "28.87 kB"
"maxSize": "29 kB"
},
{
"path": "./dist/size-rollup-animate.js",
Expand Down
6 changes: 3 additions & 3 deletions packages/framer-motion/src/animation/optimized-appear/get-appear-id.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { VisualElement } from "../../render/VisualElement"
import { optimizedAppearDataAttribute } from "./data-id"
import { WithAppearProps } from "./types"

export function getOptimisedAppearId(
visualElement: VisualElement
visualElement: WithAppearProps
): string | undefined {
return visualElement.getProps()[optimizedAppearDataAttribute]
return visualElement.props[optimizedAppearDataAttribute]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change? 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Practically it doesn't make a difference but it just made the types subset of VisualElement simpler

}
6 changes: 1 addition & 5 deletions packages/framer-motion/src/animation/optimized-appear/handoff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { Batcher } from "../../frameloop/types"
import { transformProps } from "../../render/html/utils/transform"
import { appearAnimationStore } from "./store"
import { appearStoreId } from "./store-id"

Expand All @@ -8,10 +7,7 @@ export function handoffOptimizedAppearAnimation(
valueName: string,
frame: Batcher
): number | null {
const optimisedValueName = transformProps.has(valueName)
? "transform"
: valueName
const storeId = appearStoreId(elementId, optimisedValueName)
const storeId = appearStoreId(elementId, valueName)
const optimisedAnimation = appearAnimationStore.get(storeId)

if (!optimisedAnimation) {
Expand Down
74 changes: 59 additions & 15 deletions packages/framer-motion/src/animation/optimized-appear/start.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import { handoffOptimizedAppearAnimation } from "./handoff"
import { appearAnimationStore, elementsWithAppearAnimations } from "./store"
import { noop } from "../../utils/noop"
import "./types"
import { getOptimisedAppearId } from "./get-appear-id"
import { MotionValue } from "../../value"
import type { WithAppearProps } from "./types"

/**
* A single time to use across all animations to manually set startTime
Expand Down Expand Up @@ -65,10 +68,26 @@ export function startOptimizedAppearAnimation(
*/
window.MotionHandoffAnimation = handoffOptimizedAppearAnimation

window.MotionHasOptimisedTransformAnimation = (elementId?: string) => {
window.MotionHasOptimisedAnimation = (
elementId?: string,
valueName?: string
) => {
if (!elementId) return false

const animationId = appearStoreId(elementId, "transform")
/**
* Keep a map of elementIds that have started animating. We check
* via ID instead of Element because of hydration errors and
* pre-hydration checks. We also actively record IDs as they start
* animating rather than simply checking for data-appear-id as
* this attrbute might be present but not lead to an animation, for
* instance if the element's appear animation is on a different
* breakpoint.
*/
if (!valueName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In what cases do we not provide a value name for the optimized animation?

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 this instance it will check for any optimised animation - for instance when determining if we fire animations on layout effect or effect

return elementsWithAppearAnimations.has(elementId)
}

const animationId = appearStoreId(elementId, valueName)
return Boolean(appearAnimationStore.get(animationId))
}

Expand All @@ -77,8 +96,11 @@ export function startOptimizedAppearAnimation(
* they're the ones that will interfere with the
* layout animation measurements.
*/
window.MotionCancelOptimisedTransform = (elementId: string) => {
const animationId = appearStoreId(elementId, "transform")
window.MotionCancelOptimisedAnimation = (
elementId: string,
valueName: string
) => {
const animationId = appearStoreId(elementId, valueName)
const data = appearAnimationStore.get(animationId)

if (data) {
Expand All @@ -87,17 +109,39 @@ export function startOptimizedAppearAnimation(
}
}

/**
* Keep a map of elementIds that have started animating. We check
* via ID instead of Element because of hydration errors and
* pre-hydration checks. We also actively record IDs as they start
* animating rather than simply checking for data-appear-id as
* this attrbute might be present but not lead to an animation, for
* instance if the element's appear animation is on a different
* breakpoint.
*/
window.MotionHasOptimisedAnimation = (elementId?: string) =>
Boolean(elementId && elementsWithAppearAnimations.has(elementId))
window.MotionCheckAppearSync = (
visualElement: WithAppearProps,
valueName: string,
value: MotionValue
) => {
const appearId = getOptimisedAppearId(visualElement)

if (!appearId) return

const valueIsOptimised = window.MotionHasOptimisedAnimation?.(
appearId,
valueName
)
const externalAnimationValue =
visualElement.props.values?.[valueName]

if (!valueIsOptimised || !externalAnimationValue) return

const removeSyncCheck = value.on(
"change",
(latestValue: string | number) => {
if (externalAnimationValue.get() !== latestValue) {
window.MotionCancelOptimisedAnimation?.(
appearId,
valueName
)
removeSyncCheck()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not super familiar with the value.on API, why do we need to call removeSyncCheck again?

Copy link
Collaborator Author

@mattgperry mattgperry Aug 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Think of it like

function doThing() {
  if (someCondition) {
    // stop doing thing
    element.removeEventListener("mousestart", doThing)
  }
}

element.addEventListener("mousestart", doThing)

}
}
)

return removeSyncCheck
}
}

const startAnimation = () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/framer-motion/src/animation/optimized-appear/store-id.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
export const appearStoreId = (id: string, value: string) => `${id}: ${value}`
import { transformProps } from "../../render/html/utils/transform"

export const appearStoreId = (elementId: string, valueName: string) => {
const key = transformProps.has(valueName) ? "transform" : valueName

return `${elementId}: ${key}`
}
31 changes: 28 additions & 3 deletions packages/framer-motion/src/animation/optimized-appear/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
import type { Batcher } from "../../frameloop/types"
import { MotionValue } from "../../value"
import { optimizedAppearDataAttribute } from "./data-id"

/**
* Expose only the needed part of the VisualElement interface to
* ensure React types don't end up in the generic DOM bundle.
*/
export interface WithAppearProps {
props: {
[optimizedAppearDataAttribute]?: string
values?: {
[key: string]: MotionValue<number> | MotionValue<string>
}
}
}

export type HandoffFunction = (
storeId: string,
Expand All @@ -14,8 +29,18 @@ declare global {
interface Window {
MotionHandoffAnimation?: HandoffFunction
MotionHandoffIsComplete?: boolean
MotionCancelOptimisedTransform?: (id?: string) => void
MotionHasOptimisedTransformAnimation?: (id?: string) => boolean
MotionHasOptimisedAnimation?: (id?: string) => boolean
MotionHasOptimisedAnimation?: (
elementId?: string,
valueName?: string
) => boolean
MotionCancelOptimisedAnimation?: (
elementId?: string,
valueName?: string
) => void
MotionCheckAppearSync?: (
visualElement: WithAppearProps,
valueName: string,
value: MotionValue
) => VoidFunction | void
}
}
6 changes: 3 additions & 3 deletions packages/framer-motion/src/projection/node/create-projection-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ function cancelTreeOptimisedTransformAnimations(

const appearId = getOptimisedAppearId(visualElement)

if (window.MotionHasOptimisedTransformAnimation!(appearId)) {
window.MotionCancelOptimisedTransform!(appearId)
if (window.MotionHasOptimisedAnimation!(appearId, "transform")) {
window.MotionCancelOptimisedAnimation!(appearId, "transform")
}

const { parent } = projectionNode
Expand Down Expand Up @@ -640,7 +640,7 @@ export function createProjectionNode<I>({
* if a layout animation measurement is actually going to be affected by them.
*/
if (
window.MotionCancelOptimisedTransform &&
window.MotionCancelOptimisedAnimation &&
!this.hasCheckedOptimisedAppear
) {
cancelTreeOptimisedTransformAnimations(this)
Expand Down
13 changes: 12 additions & 1 deletion packages/framer-motion/src/render/VisualElement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,11 @@ export abstract class VisualElement<
this.removeFromVariantTree = this.parent.addVariantChild(this)
}

this.values.forEach((value, key) => this.bindToMotionValue(key, value))
this.values.forEach((value, key) => {
if (!this.valueSubscriptions.has(key)) {
this.bindToMotionValue(key, value)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I also don't fully understand this change, as bindToMotionValue seems to be designed to handle cases where a value subscription is present as well. 🤔

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 good catch - this is a legacy from fixing the same bug that other check fixed (that I discovered while making this PR)

})

if (!hasReducedMotionListener.current) {
initPrefersReducedMotion()
Expand Down Expand Up @@ -427,6 +431,7 @@ export abstract class VisualElement<
cancelFrame(this.notifyUpdate)
cancelFrame(this.render)
this.valueSubscriptions.forEach((remove) => remove())
this.valueSubscriptions.clear()
this.removeFromVariantTree && this.removeFromVariantTree()
this.parent && this.parent.children.delete(this)

Expand Down Expand Up @@ -469,9 +474,15 @@ export abstract class VisualElement<
this.scheduleRender
)

let removeSyncCheck: VoidFunction | void
if (window.MotionCheckAppearSync) {
removeSyncCheck = window.MotionCheckAppearSync(this, key, value)
}

this.valueSubscriptions.set(key, () => {
removeOnChange()
removeOnRenderRequest()
if (removeSyncCheck) removeSyncCheck()
if (value.owner) value.stop()
})
}
Expand Down