Skip to content
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
7 changes: 7 additions & 0 deletions .changeset/animated-attribute-removal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@react-spring/web': patch
---

fix(web): remove DOM attributes when their animated value becomes `undefined`

Previously, attributes such as `inert`, `disabled`, `viewBox`, `className`, and `children` were coerced to the string `"undefined"` or left stale when their animated value resolved to `undefined`. Boolean-style attributes like `inert` must be entirely removed to be disabled — setting them to any value (including `"undefined"`) keeps them active. `applyAnimatedValues` now calls `removeAttribute` (or clears the class/textContent) in this case.
85 changes: 84 additions & 1 deletion targets/web/src/animated.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react'
import { forwardRef } from 'react'
import { render } from 'vitest-browser-react'
import createMockRaf, { MockRaf } from '@react-spring/mock-raf'
import { Globals } from '@react-spring/shared'
import { FluidValue, Globals, callFluidObservers } from '@react-spring/shared'
import { SpringValue, Animatable } from '@react-spring/core'

import { a } from './index'
Expand Down Expand Up @@ -224,6 +224,89 @@ describe('animated component', () => {
})
})

describe('animated component attribute removal', () => {
it('removes a boolean-style attribute when its animated value becomes undefined', () => {
const inert = new TestFluid<true | undefined>(true)
const { getByTestId } = render(
<a.div inert={inert as any} data-testid="wrapper" />
)
const el = getByTestId('wrapper').element() as HTMLElement
expect(el.hasAttribute('inert')).toBe(true)
inert.set(undefined)
mockRaf.step()
expect(el.hasAttribute('inert')).toBe(false)
})
it('removes a generic attribute when its animated value becomes undefined', () => {
const value = new TestFluid<string | undefined>('bar')
const { getByTestId } = render(
<a.div data-foo={value as any} data-testid="wrapper" />
)
const el = getByTestId('wrapper').element() as HTMLElement
expect(el.getAttribute('data-foo')).toBe('bar')
value.set(undefined)
mockRaf.step()
expect(el.hasAttribute('data-foo')).toBe(false)
})
it('removes the viewBox attribute when its animated value becomes undefined', () => {
const viewBox = new TestFluid<string | undefined>('0 0 100 100')
const { getByTestId } = render(
<a.svg viewBox={viewBox as any} data-testid="wrapper" />
)
const el = getByTestId('wrapper').element() as unknown as SVGSVGElement
expect(el.getAttribute('viewBox')).toBe('0 0 100 100')
viewBox.set(undefined)
mockRaf.step()
expect(el.hasAttribute('viewBox')).toBe(false)
})
it('removes the class attribute when className becomes undefined', () => {
const className = new TestFluid<string | undefined>('initial')
const { getByTestId } = render(
<a.div className={className as any} data-testid="wrapper" />
)
const el = getByTestId('wrapper').element() as HTMLElement
expect(el.getAttribute('class')).toBe('initial')
className.set(undefined)
mockRaf.step()
expect(el.hasAttribute('class')).toBe(false)
})
it('clears textContent when children becomes undefined', () => {
const children = new TestFluid<string | undefined>('hello')
const { getByTestId } = render(
<a.div data-testid="wrapper">{children as any}</a.div>
)
const el = getByTestId('wrapper').element() as HTMLElement
expect(el.textContent).toBe('hello')
children.set(undefined)
mockRaf.step()
expect(el.textContent).toBe('')
})
it('still applies defined falsy attribute values rather than removing them', () => {
const value = new TestFluid<string>('1')
const { getByTestId } = render(
<a.div tabIndex={value as any} data-testid="wrapper" />
)
const el = getByTestId('wrapper').element() as HTMLElement
expect(el.getAttribute('tabindex')).toBe('1')
value.set('0')
mockRaf.step()
expect(el.getAttribute('tabindex')).toBe('0')
expect(el.hasAttribute('tabindex')).toBe(true)
})
})

function spring<T>(value: T): SpringValue<Animatable<T>> {
return new SpringValue(value!)
}

class TestFluid<T> extends FluidValue<T> {
constructor(private _value: T) {
super()
}
protected get(): T {
return this._value
}
set(next: T): void {
this._value = next
callFluidObservers(this, { type: 'change', parent: this })
}
}
31 changes: 24 additions & 7 deletions targets/web/src/applyAnimatedValues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ const attributeCache: Lookup<string> = {}
type Instance = HTMLDivElement & { style?: Lookup }

export function applyAnimatedValues(instance: Instance, props: Lookup) {
if (!instance.nodeType || !instance.setAttribute) {
if (
!instance.nodeType ||
!instance.setAttribute ||
!instance.removeAttribute
) {
return false
}

Expand Down Expand Up @@ -52,7 +56,7 @@ export function applyAnimatedValues(instance: Instance, props: Lookup) {
))
)

if (children !== void 0) {
if (props.hasOwnProperty('children')) {
instance.textContent = children
}

Expand All @@ -70,20 +74,33 @@ export function applyAnimatedValues(instance: Instance, props: Lookup) {

// Apply DOM attributes
names.forEach((name, i) => {
instance.setAttribute(name, values[i])
const value = values[i]
if (value !== void 0) {
instance.setAttribute(name, value)
} else {
instance.removeAttribute(name)
}
})

if (className !== void 0) {
instance.className = className
if (props.hasOwnProperty('className')) {
if (className !== void 0) {
instance.className = className
} else {
instance.removeAttribute('class')
}
}
if (scrollTop !== void 0) {
instance.scrollTop = scrollTop
}
if (scrollLeft !== void 0) {
instance.scrollLeft = scrollLeft
}
if (viewBox !== void 0) {
instance.setAttribute('viewBox', viewBox)
if (props.hasOwnProperty('viewBox')) {
if (viewBox !== void 0) {
instance.setAttribute('viewBox', viewBox)
} else {
instance.removeAttribute('viewBox')
}
}
}

Expand Down
Loading