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

Feat/html content #71

Merged
merged 6 commits into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 28 additions & 0 deletions docs/components/demo/html-content/index.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<script setup lang="ts">
import { RowValue, animate, useMotionValue, useTransform } from 'motion-v'
import { onUnmounted } from 'vue'

const count = useMotionValue(0)
const rounded = useTransform(() => Math.round(count.get()))

let controls: any
watchEffect((cleanup) => {
controls = animate(count, 100, { duration: 5 })
cleanup(() => {
controls?.stop()
})
})

onUnmounted(() => {
controls?.stop()
})
</script>

<template>
<pre
class="!bg-transparent flex items-center justify-center"
style="font-size: 64px; color: #4ff0b7;"
>
<RowValue :value="rounded" />
</pre>
</template>
6 changes: 3 additions & 3 deletions docs/components/demo/radix-dialog/index.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script setup lang="ts">
import { DialogContent, DialogDescription, DialogOverlay, DialogPortal, DialogRoot, DialogTitle, DialogTrigger } from 'radix-vue'
import { AnimatePresence, Motion, motion } from 'motion-v'
import { AnimatePresence, motion } from 'motion-v'
</script>

<template>
Expand All @@ -25,7 +25,7 @@ import { AnimatePresence, Motion, motion } from 'motion-v'
as-child
class="w-full max-w-md rounded-xl bg-white p-6 backdrop-blur-2xl z-[1001] fixed top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2"
>
<Motion
<motion.div
:initial="{ opacity: 0, scale: 0.95, x: '-50%', y: '-50%' }"
:animate="{ opacity: 1, scale: 1 }"
:exit="{ opacity: 0, scale: 0.95 }"
Expand All @@ -51,7 +51,7 @@ import { AnimatePresence, Motion, motion } from 'motion-v'
Got it, thanks!
</UiButton>
</DialogTrigger>
</Motion>
</motion.div>
</DialogContent>
</AnimatePresence>
</DialogPortal>
Expand Down
31 changes: 31 additions & 0 deletions docs/components/demo/unwrap-element/index.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<script setup lang="ts">
import { AnimatePresence, motion } from 'motion-v'
import { Popover } from 'radix-vue/namespaced'
</script>

<template>
<div class="px-4">
<Popover.Root>
<Popover.Trigger class="cursor-pointer">
Toggle popover
</Popover.Trigger>
<Popover.Portal>
<AnimatePresence :unwrap-element="true">
<Popover.Content
as-child
class="px-4 py-2 bg-pink-400"
>
<motion.div
:initial="{ opacity: 0, scale: 0.95 }"
:animate="{ opacity: 1, scale: 1 }"
:exit="{ opacity: 0, scale: 0.95 }"
:transition="{ duration: 0.3, ease: 'easeInOut' }"
>
I'm a popover!
</motion.div>
</Popover.Content>
</AnimatePresence>
</Popover.Portal>
</Popover.Root>
</div>
</template>
4 changes: 2 additions & 2 deletions docs/components/layout/AsideTreeItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ const folderStyle = computed(() => link.sidebar?.style ?? defaultFolderStyle)
<NuxtLink
v-else
:to="link._path"
class="flex items-center gap-2 rounded-md p-2 text-sm text-foreground/80 hover:bg-muted hover:text-primary"
class="flex hover:bg-primary/10 items-center gap-2 rounded-md p-2 text-sm text-foreground/80 hover:text-primary"
:class="[
isActive && 'bg-muted !text-primary',
isActive && ' !text-primary bg-primary/10',
link.navTruncate !== false && 'h-8',
]"
>
Expand Down
8 changes: 8 additions & 0 deletions docs/content/2.components/2.animate-presence.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,11 @@ Decides how AnimatePresence handles entering and exiting children.
- `popLayout`: Exiting children will be "popped" out of the page layout. This allows surrounding elements to move to their new layout immediately.

<ComponentPreview name="pop-layout" />

### `unwrapElement`

- Default: `false`

When `true`, `AnimatePresence` will use the first child element as the transition target instead of the wrapper element. This is useful when working with components like Radix UI's PopoverContent that render an additional wrapper element.

<ComponentPreview name="unwrap-element" />
4 changes: 4 additions & 0 deletions docs/content/4.motion-value/6.use-transform.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ useTransform(x, [0, 100], ['#f00', '00f'])

<ComponentPreview name="drag-transform" />

## HTML Content

<ComponentPreview name="html-content" />

<iframe src="https://stackblitz.com/edit/vitejs-vite-ff3czw?ctl=1&embed=1&file=src%2FApp.vue&hideExplorer=1"
style="width:100%; height: 500px; border:0; border-radius: 4px; overflow:hidden;"
title="motion-use-spring"
Expand Down
22 changes: 22 additions & 0 deletions packages/motion/src/components/RowValue.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<script setup generic="T" lang="ts">
import type { MotionValue } from 'framer-motion/dom'
import { getCurrentInstance, watchEffect } from 'vue'

const props = defineProps<{
value: MotionValue<T>
}>()

const instance = getCurrentInstance().proxy
watchEffect((cleanup) => {
const unSub = props.value.on('change', (value) => {
if (instance.$el) {
instance.$el.textContent = value
}
})
cleanup(unSub)
})
</script>

<template>
{{ value.get() }}
</template>
81 changes: 81 additions & 0 deletions packages/motion/src/components/__tests__/row-value.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { describe, expect, it, vi } from 'vitest'
import { mount } from '@vue/test-utils'
import { motionValue } from 'framer-motion/dom'
import RowValue from '../RowValue.vue'

describe('row-value', () => {
it('should render initial value', () => {
const value = motionValue(10)
const wrapper = mount(RowValue, {
props: {
value,
},
})

expect(wrapper.text()).toBe('10')
})

it('should update when value changes', async () => {
const value = motionValue('initial')
const wrapper = mount(RowValue, {
props: {
value,
},
})

expect(wrapper.text()).toBe('initial')

value.set('updated')
await wrapper.vm.$nextTick()

expect(wrapper.text()).toBe('updated')
})

it('should cleanup subscription on unmount', async () => {
const value = motionValue('test')
const unsubscribe = vi.fn()
vi.spyOn(value, 'on').mockImplementation(() => unsubscribe)

const wrapper = mount(RowValue, {
props: {
value,
},
})

await wrapper.unmount()
expect(unsubscribe).toHaveBeenCalled()
})

it('should handle different value types', () => {
const numberValue = motionValue(42)
const numberWrapper = mount(RowValue, {
props: { value: numberValue },
})
expect(numberWrapper.text()).toBe('42')

const stringValue = motionValue('hello')
const stringWrapper = mount(RowValue, {
props: { value: stringValue },
})
expect(stringWrapper.text()).toBe('hello')

const boolValue = motionValue(true)
const boolWrapper = mount(RowValue, {
props: { value: boolValue },
})
expect(boolWrapper.text()).toBe('true')
})

it('should update DOM element directly when value changes', async () => {
const value = motionValue('initial')
const wrapper = mount(RowValue, {
props: { value },
})

const el = wrapper.element
value.set('updated via DOM')
await wrapper.vm.$nextTick()

expect(el.textContent).toBe('updated via DOM')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const props = withDefaults(defineProps<AnimatePresenceProps>(), {
mode: 'sync',
initial: true,
multiple: false,
unwrapElement: false,
})

const presenceContext = {
Expand Down Expand Up @@ -49,9 +50,13 @@ onUnmounted(() => {
})
// 处理元素退出动画
function exit(el: Element, done: VoidFunction) {
const state = mountedStates.get(el)
let state = mountedStates.get(el)
if (!state) {
return done()
if (!props.unwrapElement) {
return done()
}
el = el.firstElementChild as Element
state = mountedStates.get(el)
}
exitDom.set(el, true)
removeDoneCallback(el)
Expand Down
1 change: 1 addition & 0 deletions packages/motion/src/components/animate-presence/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ export interface AnimatePresenceProps {
as?: string

Choose a reason for hiding this comment

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

The property as is ambiguously named, which might cause confusion about its purpose. It's better to use a more descriptive name that clearly indicates its role, such as elementType or componentType, especially if it's used to specify the type of component or HTML tag to render.

Recommended Change:

elementType?: string

custom?: any

Choose a reason for hiding this comment

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

The custom property is currently typed as any, which can lead to potential type safety issues as it bypasses TypeScript's static type checking. It's recommended to define a more specific type or at least use unknown if the type is not determinable, which would enforce type checking at the usage points.

Recommended Change:

custom?: unknown

onExitComplete?: VoidFunction
unwrapElement?: boolean
}
2 changes: 1 addition & 1 deletion packages/motion/src/components/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function nodeGroup(): NodeGroup {
unsubscribe()
subscriptions.delete(node)
}
Comment on lines 31 to 33

Choose a reason for hiding this comment

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

Lack of Error Handling

The unsubscribe function (lines 31-33) is called without any error handling. If this function throws an error, it could lead to unhandled exceptions which might crash the application or lead to unexpected behavior.

Recommendation:
Wrap the unsubscribe call in a try-catch block to handle potential errors gracefully. Log the error or handle it according to the application's error management strategy.

// dirtyAll()
dirtyAll()

Choose a reason for hiding this comment

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

Performance Concern

Calling dirtyAll on every node removal (line 34) can lead to performance issues in scenarios where the node set is large. This is because it triggers the notify function for all nodes, which might be computationally expensive.

Recommendation:
Consider optimizing this by determining if all nodes need to be notified upon the removal of one node. If not, modify the logic to selectively update nodes based on dependency or impact, or debounce the updates if multiple removals are expected in short sequences.

},
dirty: dirtyAll,
}
Expand Down
1 change: 1 addition & 0 deletions packages/motion/src/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export { Motion, motion, type MotionProps } from './motion'
export * from './animate-presence'
export * from './motion-config'
Comment on lines 2 to 3

Choose a reason for hiding this comment

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

Using wildcard exports (export * from ...) can lead to unintentional exposure of internal modules and makes it difficult to track dependencies. This can compromise the maintainability and security of the codebase.

Recommendation: Explicitly list the exports from each module. This approach improves clarity, makes the code easier to refactor, and helps in maintaining a clear API surface.

export * from './reorder'
export { default as RowValue } from './RowValue.vue'
2 changes: 1 addition & 1 deletion packages/motion/src/components/reorder/Group.vue
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ reorderContextProvider({
const newOrder = checkReorder(order, item, offset, velocity)
if (order !== newOrder) {
isReordering = true
props['onUpdate:values'](
props['onUpdate:values']?.(
newOrder
.map(getValue)
.filter(value => props.values.includes(value)),
Expand Down
17 changes: 9 additions & 8 deletions packages/motion/src/features/layout/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,17 @@ export class LayoutFeature extends Feature {
this.state.visualElement.projection?.root?.didUpdate()
}

beforeMount() {
}

mount() {
const options = this.state.options
const layoutGroup = this.state.options.layoutGroup
if (options.layout || options.layoutId) {
if (options.layout || options.layoutId || options.drag) {
const projection = this.state.visualElement.projection

if (projection) {
projection.promote()
layoutGroup?.group?.add(projection)
}
globalProjectionState.hasEverUpdated = true
this.didUpdate()
globalProjectionState.hasEverUpdated = true

Choose a reason for hiding this comment

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

Direct manipulation of a global state (globalProjectionState.hasEverUpdated = true) within the mount method can lead to unintended side effects if this state is shared across multiple components or parts of the application. Consider encapsulating this state change within a method or using a state management pattern that isolates side effects and makes the state changes more predictable.

}
}

Expand All @@ -57,8 +54,12 @@ export class LayoutFeature extends Feature {
unmount() {
const layoutGroup = this.state.options.layoutGroup
const projection = this.state.visualElement.projection
if (layoutGroup?.group && projection)

if (layoutGroup?.group && projection) {
layoutGroup.group.remove(projection)
this.didUpdate()
}
else {
this.didUpdate()
}
Comment on lines +61 to +63

Choose a reason for hiding this comment

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

The else block in the unmount method calls didUpdate() unconditionally, which could lead to unexpected behavior if didUpdate() has side effects or if it's not intended to be called in this context. It's recommended to add specific conditions or checks before calling such methods, or to ensure that the method itself handles being called in this context appropriately.

}
}
Loading