Skip to content

Commit 83b08cc

Browse files
committed
fix: prevent slider bubble input mount cycles
1 parent e389904 commit 83b08cc

2 files changed

Lines changed: 61 additions & 33 deletions

File tree

packages/slider/src/slider.tsx

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -731,10 +731,7 @@ function SliderThumb(props: ScopedProps<SliderThumbProps>): FictNode {
731731
const dimension = size()?.[orientation.size()]
732732
return dimension ? getThumbInBoundsOffset(dimension, percent(), orientation.direction()) : 0
733733
}
734-
const isFormControl = () => {
735-
const thumbNode = thumb()
736-
return thumbNode ? Boolean(context.form() || thumbNode.closest('form')) : true
737-
}
734+
const isFormControl = createSignal(false)
738735
const label = () => getLabel(index(), context.values().length)
739736
const wrapperStyle = () => ({
740737
transform: 'var(--radix-slider-thumb-transform)',
@@ -773,28 +770,31 @@ function SliderThumb(props: ScopedProps<SliderThumbProps>): FictNode {
773770
const thumbNode = thumb()
774771
if (!thumbNode) return
775772

773+
isFormControl(Boolean(context.form() || thumbNode.closest('form')))
776774
context.thumbs.add(thumbNode)
777775
return () => {
776+
isFormControl(false)
778777
context.thumbs.delete(thumbNode)
779778
}
780779
})
781780

782-
const bubbleInputNode = (isFormControl() ? (
783-
<SliderBubbleInput
784-
__scopeSlider={props.__scopeSlider}
785-
name={() => {
786-
const itemName =
787-
props.name === undefined
788-
? context.name()
789-
: readValue(props.name as MaybeAccessor<string | undefined>)
790-
791-
if (!itemName) return undefined
792-
return context.values().length > 1 ? `${itemName}[]` : itemName
793-
}}
794-
form={context.form}
795-
value={value}
796-
/>
797-
) : null) as unknown as FictNode
781+
const bubbleInputNode = (() =>
782+
isFormControl() ? (
783+
<SliderBubbleInput
784+
__scopeSlider={props.__scopeSlider}
785+
name={() => {
786+
const itemName =
787+
props.name === undefined
788+
? context.name()
789+
: readValue(props.name as MaybeAccessor<string | undefined>)
790+
791+
if (!itemName) return undefined
792+
return context.values().length > 1 ? `${itemName}[]` : itemName
793+
}}
794+
form={context.form}
795+
value={value}
796+
/>
797+
) : null) as unknown as FictNode
798798

799799
return (
800800
<span style={wrapperStyle()}>
@@ -823,28 +823,28 @@ function SliderBubbleInput(props: ScopedProps<SliderBubbleInputProps>): FictNode
823823
const input = ref()
824824
const nextValue = readValue(props.value)
825825
const prevValue = previousValue()
826+
const nextValueString = nextValue === undefined ? '' : String(nextValue)
826827

827828
if (!input) return
828829

829-
if (!Object.is(prevValue, nextValue)) {
830-
const descriptor = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, 'value')
831-
const setValue = descriptor?.set
830+
const descriptor = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, 'value')
831+
const setValue = descriptor?.set
832+
if (input.value !== nextValueString) {
832833
if (setValue) {
833-
setValue.call(input, nextValue === undefined ? '' : String(nextValue))
834+
setValue.call(input, nextValueString)
834835
} else {
835-
input.value = nextValue === undefined ? '' : String(nextValue)
836+
input.value = nextValueString
836837
}
838+
}
837839

840+
if (!Object.is(prevValue, nextValue)) {
838841
input.dispatchEvent(new Event('input', { bubbles: true }))
839842
}
840843
})
841844

842845
const inputProps = mergeProps(() => inputRestProps as Record<string, unknown>, {
843846
__scopeSlider: undefined,
844-
defaultValue: prop(() => {
845-
const nextValue = readValue(props.value)
846-
return nextValue === undefined ? '' : String(nextValue)
847-
}),
847+
defaultValue: undefined,
848848
'attr:form': prop(() =>
849849
formProp === undefined ? undefined : readValue(formProp as MaybeAccessor<string | undefined>),
850850
),
@@ -858,7 +858,10 @@ function SliderBubbleInput(props: ScopedProps<SliderBubbleInputProps>): FictNode
858858
display: 'none',
859859
...readStyle(props.style),
860860
})),
861-
value: undefined,
861+
value: prop(() => {
862+
const nextValue = readValue(props.value)
863+
return nextValue === undefined ? '' : String(nextValue)
864+
}),
862865
})
863866

864867
return (

packages/slider/test/index.test.tsx

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe('@fictjs/slider', () => {
5858
document.body.innerHTML = ''
5959
})
6060

61-
it('updates a single-thumb slider from pointer interactions and mirrors the value to the bubble input', async () => {
61+
it('updates a single-thumb slider from pointer interactions without rendering a bubble input outside forms', async () => {
6262
const container = document.createElement('div')
6363
document.body.append(container)
6464
const onValueCommit = vi.fn()
@@ -79,7 +79,6 @@ describe('@fictjs/slider', () => {
7979

8080
const slider = container.querySelector('[data-orientation="horizontal"]') as HTMLSpanElement
8181
const range = container.querySelector('[data-testid="range"]') as HTMLSpanElement
82-
const bubbleInput = container.querySelector('input') as HTMLInputElement
8382

8483
Object.defineProperty(slider, 'getBoundingClientRect', {
8584
value: () =>
@@ -103,10 +102,36 @@ describe('@fictjs/slider', () => {
103102

104103
expect(range.style.left).toBe('0%')
105104
expect(range.style.right).toBe('50%')
106-
expect(bubbleInput.value).toBe('50')
105+
expect(container.querySelector('input')).toBeNull()
107106
expect(onValueCommit).toHaveBeenCalledWith([50])
108107
})
109108

109+
it('renders a bubble input with the current value when the slider participates in a form', async () => {
110+
const container = document.createElement('div')
111+
document.body.append(container)
112+
113+
mount(
114+
() => (
115+
<form>
116+
<Root defaultValue={[25]} max={100} name="volume">
117+
<Track data-testid="track">
118+
<Range data-testid="range" />
119+
</Track>
120+
<Thumb />
121+
</Root>
122+
</form>
123+
),
124+
container,
125+
)
126+
127+
await waitForEffects()
128+
129+
const bubbleInput = container.querySelector('input') as HTMLInputElement
130+
131+
expect(bubbleInput.name).toBe('volume')
132+
expect(bubbleInput.value).toBe('25')
133+
})
134+
110135
it('steps the focused thumb with keyboard input', async () => {
111136
const container = document.createElement('div')
112137
document.body.append(container)

0 commit comments

Comments
 (0)