From e3531aca60d40802ea30fac8e1bc607e71d6129d Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 15 Jul 2022 22:01:25 +0200 Subject: [PATCH 1/5] implement uncontrolled form components A few versions ago we introduced compatibility with the native `form` element. This means that behind the scenes we render hidden inputs that are kept in sync which allows you to submit your normal form and get data via `new FormData(event.currentTarget)`. Before this change every form related component (Switch, RadioGroup, Listbox and Combobox) always had to be passed a `value` and an `onChange` regardless of this change. This change will allow you to not even use the `value` and the `onChange` at all and keep it completely uncontrolled. This has some changes: - `value` is made optional - `onChange` is made optional (but will still be called if passed regardless of being controlled or uncontrolled) - `defaultValue` got added so that you can still pre-fill your values with known values. - `value` render prop got exposed so that you can still use this while rendering. This should also make it completely compatible with tools like Remix without wiring up your own state. --- .../src/components/combobox/combobox.tsx | 21 ++++++++----- .../src/components/listbox/listbox.tsx | 19 +++++++----- .../components/radio-group/radio-group.tsx | 30 +++++++++++++++---- .../src/components/switch/switch.tsx | 21 +++++++++---- .../src/hooks/use-controllable.ts | 23 ++++++++++++++ 5 files changed, 88 insertions(+), 26 deletions(-) create mode 100644 packages/@headlessui-react/src/hooks/use-controllable.ts diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index c7e0c73b2..1c3bc91f0 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -40,6 +40,7 @@ import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed' import { Keys } from '../keyboard' +import { useControllable } from '../../hooks/use-controllable' enum ComboboxState { Open, @@ -311,10 +312,11 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< props: Props< TTag, ComboboxRenderPropArg, - 'value' | 'onChange' | 'disabled' | 'name' | 'nullable' | 'multiple' | 'by' + 'value' | 'defaultValue' | 'onChange' | 'by' | 'disabled' | 'name' | 'nullable' | 'multiple' > & { - value: TType - onChange(value: TType): void + value?: TType + defaultValue?: TType + onChange?(value: TType): void by?: (keyof TActualType & string) | ((a: TActualType, z: TActualType) => boolean) disabled?: boolean __demoMode?: boolean @@ -325,9 +327,10 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< ref: Ref ) { let { + value: controlledValue, + defaultValue, + onChange: controlledOnChange, name, - value, - onChange: theirOnChange, by = (a, z) => a === z, disabled = false, __demoMode = false, @@ -335,6 +338,7 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< multiple = false, ...theirProps } = props + let [value, theirOnChange] = useControllable(controlledValue, controlledOnChange, defaultValue) let [state, dispatch] = useReducer(stateReducer, { dataRef: createRef(), @@ -430,8 +434,9 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< data.activeOptionIndex === null ? null : (data.options[data.activeOptionIndex].dataRef.current.value as TType), + value, }), - [data, disabled] + [data, disabled, value] ) let syncInputValue = useCallback(() => { @@ -495,7 +500,7 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< let onChange = useEvent((value: unknown) => { return match(data.mode, { [ValueMode.Single]() { - return theirOnChange(value as TType) + return theirOnChange?.(value as TType) }, [ValueMode.Multi]() { let copy = (data.value as TActualType[]).slice() @@ -507,7 +512,7 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< copy.splice(idx, 1) } - return theirOnChange(copy as unknown as TType) + return theirOnChange?.(copy as unknown as TType) }, }) }) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 974cb38e4..8efd81139 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -37,6 +37,7 @@ import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { objectToFormEntries } from '../../utils/form' import { getOwnerDocument } from '../../utils/owner' import { useEvent } from '../../hooks/use-event' +import { useControllable } from '../../hooks/use-controllable' enum ListboxStates { Open, @@ -311,10 +312,11 @@ let ListboxRoot = forwardRefWithAs(function Listbox< props: Props< TTag, ListboxRenderPropArg, - 'value' | 'onChange' | 'disabled' | 'horizontal' | 'name' | 'multiple' | 'by' + 'value' | 'defaultValue' | 'onChange' | 'by' | 'disabled' | 'horizontal' | 'name' | 'multiple' > & { - value: TType - onChange(value: TType): void + value?: TType + defaultValue?: TType + onChange?(value: TType): void by?: (keyof TActualType & string) | ((a: TActualType, z: TActualType) => boolean) disabled?: boolean horizontal?: boolean @@ -324,9 +326,10 @@ let ListboxRoot = forwardRefWithAs(function Listbox< ref: Ref ) { let { - value, + value: controlledValue, + defaultValue, name, - onChange, + onChange: controlledOnChange, by = (a, z) => a === z, disabled = false, horizontal = false, @@ -336,6 +339,8 @@ let ListboxRoot = forwardRefWithAs(function Listbox< const orientation = horizontal ? 'horizontal' : 'vertical' let listboxRef = useSyncRefs(ref) + let [value, onChange] = useControllable(controlledValue, controlledOnChange, defaultValue) + let reducerBag = useReducer(stateReducer, { listboxState: ListboxStates.Closed, propsRef: { @@ -413,8 +418,8 @@ let ListboxRoot = forwardRefWithAs(function Listbox< ) let slot = useMemo( - () => ({ open: listboxState === ListboxStates.Open, disabled }), - [listboxState, disabled] + () => ({ open: listboxState === ListboxStates.Open, disabled, value }), + [listboxState, disabled, value] ) let ourProps = { ref: listboxRef } diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx index cd848b76f..a1b82a50b 100644 --- a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx +++ b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx @@ -29,6 +29,7 @@ import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { attemptSubmit, objectToFormEntries } from '../../utils/form' import { getOwnerDocument } from '../../utils/owner' import { useEvent } from '../../hooks/use-event' +import { useControllable } from '../../hooks/use-controllable' interface Option { id: string @@ -103,7 +104,9 @@ function stateReducer(state: StateDefinition, action: Actions) { // --- let DEFAULT_RADIO_GROUP_TAG = 'div' as const -interface RadioGroupRenderPropArg {} +interface RadioGroupRenderPropArg { + value: TType +} type RadioGroupPropsWeControl = 'role' | 'aria-labelledby' | 'aria-describedby' | 'id' let RadioGroupRoot = forwardRefWithAs(function RadioGroup< @@ -112,18 +115,27 @@ let RadioGroupRoot = forwardRefWithAs(function RadioGroup< >( props: Props< TTag, - RadioGroupRenderPropArg, + RadioGroupRenderPropArg, RadioGroupPropsWeControl | 'value' | 'onChange' | 'disabled' | 'name' | 'by' > & { - value: TType - onChange(value: TType): void + value?: TType + defaultValue?: TType + onChange?(value: TType): void by?: (keyof TType & string) | ((a: TType, z: TType) => boolean) disabled?: boolean name?: string }, ref: Ref ) { - let { value, name, onChange, by = (a, z) => a === z, disabled = false, ...theirProps } = props + let { + value: controlledValue, + defaultValue, + name, + onChange: controlledOnChange, + by = (a, z) => a === z, + disabled = false, + ...theirProps + } = props let compare = useEvent( typeof by === 'string' ? (a: TType, z: TType) => { @@ -140,6 +152,8 @@ let RadioGroupRoot = forwardRefWithAs(function RadioGroup< let internalRadioGroupRef = useRef(null) let radioGroupRef = useSyncRefs(internalRadioGroupRef, ref) + let [value, onChange] = useControllable(controlledValue, controlledOnChange, defaultValue) + let firstOption = useMemo( () => options.find((option) => { @@ -161,7 +175,8 @@ let RadioGroupRoot = forwardRefWithAs(function RadioGroup< )?.propsRef.current if (nextOption?.disabled) return false - onChange(nextValue) + onChange?.(nextValue) + return true }) @@ -266,6 +281,8 @@ let RadioGroupRoot = forwardRefWithAs(function RadioGroup< onKeyDown: handleKeyDown, } + let slot = useMemo>(() => ({ value }), [value]) + return ( @@ -290,6 +307,7 @@ let RadioGroupRoot = forwardRefWithAs(function RadioGroup< {render({ ourProps, theirProps, + slot, defaultTag: DEFAULT_RADIO_GROUP_TAG, name: 'RadioGroup', })} diff --git a/packages/@headlessui-react/src/components/switch/switch.tsx b/packages/@headlessui-react/src/components/switch/switch.tsx index 7a44ea12e..4a4110771 100644 --- a/packages/@headlessui-react/src/components/switch/switch.tsx +++ b/packages/@headlessui-react/src/components/switch/switch.tsx @@ -25,6 +25,7 @@ import { useSyncRefs } from '../../hooks/use-sync-refs' import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { attemptSubmit } from '../../utils/form' import { useEvent } from '../../hooks/use-event' +import { useControllable } from '../../hooks/use-controllable' interface StateDefinition { switch: HTMLButtonElement | null @@ -101,16 +102,24 @@ let SwitchRoot = forwardRefWithAs(function Switch< props: Props< TTag, SwitchRenderPropArg, - SwitchPropsWeControl | 'checked' | 'onChange' | 'name' | 'value' + SwitchPropsWeControl | 'checked' | 'defaultChecked' | 'onChange' | 'name' | 'value' > & { - checked: boolean - onChange(checked: boolean): void + checked?: boolean + defaultChecked?: boolean + onChange?(checked: boolean): void name?: string value?: string }, ref: Ref ) { - let { checked, onChange, name, value, ...theirProps } = props + let { + checked: controlledChecked, + defaultChecked = false, + onChange: controlledOnChange, + name, + value, + ...theirProps + } = props let id = `headlessui-switch-${useId()}` let groupContext = useContext(GroupContext) let internalSwitchRef = useRef(null) @@ -121,7 +130,9 @@ let SwitchRoot = forwardRefWithAs(function Switch< groupContext === null ? null : groupContext.setSwitch ) - let toggle = useEvent(() => onChange(!checked)) + let [checked, onChange] = useControllable(controlledChecked, controlledOnChange, defaultChecked) + + let toggle = useEvent(() => onChange?.(!checked)) let handleClick = useEvent((event: ReactMouseEvent) => { if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault() event.preventDefault() diff --git a/packages/@headlessui-react/src/hooks/use-controllable.ts b/packages/@headlessui-react/src/hooks/use-controllable.ts new file mode 100644 index 000000000..17da50670 --- /dev/null +++ b/packages/@headlessui-react/src/hooks/use-controllable.ts @@ -0,0 +1,23 @@ +import { useState } from 'react' +import { useEvent } from './use-event' + +export function useControllable( + controlledValue: T | undefined, + onChange?: (value: T) => void, + defaultValue?: T +) { + let [internalValue, setInternalValue] = useState(defaultValue) + let isControlled = controlledValue !== undefined + + return [ + (isControlled ? controlledValue : internalValue)!, + useEvent((value) => { + if (isControlled) { + return onChange?.(value) + } else { + setInternalValue(value) + return onChange?.(value) + } + }), + ] as const +} From 0c41963c1b866a6b103e48c97512d78a9c27a798 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 15 Jul 2022 22:07:29 +0200 Subject: [PATCH 2/5] update example combinations/form playground to use uncontrolled components --- .../pages/combinations/form.tsx | 160 +++++++++--------- 1 file changed, 78 insertions(+), 82 deletions(-) diff --git a/packages/playground-react/pages/combinations/form.tsx b/packages/playground-react/pages/combinations/form.tsx index 99bc2f448..1b34c2e30 100644 --- a/packages/playground-react/pages/combinations/form.tsx +++ b/packages/playground-react/pages/combinations/form.tsx @@ -23,12 +23,6 @@ export default function App() { let [result, setResult] = useState(() => typeof window === 'undefined' || typeof document === 'undefined' ? [] : new FormData() ) - let [notifications, setNotifications] = useState(false) - let [apple, setApple] = useState(false) - let [banana, setBanana] = useState(false) - let [size, setSize] = useState(sizes[0]) - let [person, setPerson] = useState(people[0]) - let [activeLocation, setActiveLocation] = useState(locations[0]) let [query, setQuery] = useState('') return ( @@ -47,8 +41,7 @@ export default function App() { Enable notifications classNames( @@ -74,8 +67,6 @@ export default function App() { Apple @@ -99,8 +90,6 @@ export default function App() { Banana @@ -123,7 +112,7 @@ export default function App() {
- +
{sizes.map((size) => { return ( @@ -177,75 +166,83 @@ export default function App() {
- -
- - - {person.name.first} - - - - + + {({ value }) => ( + <> +
+ + + {value?.name?.first} + + + + + + - - -
- - {people.map((person) => ( - { - return classNames( - 'relative cursor-default select-none py-2 pl-3 pr-9 ', - active ? 'bg-blue-600 text-white' : 'text-gray-900' - ) - }} - > - {({ active, selected }) => ( - <> - - {person.name.first} - - {selected && ( - + + {people.map((person) => ( + { + return classNames( + 'relative cursor-default select-none py-2 pl-3 pr-9 ', + active ? 'bg-blue-600 text-white' : 'text-gray-900' + ) + }} + > + {({ active, selected }) => ( + <> + + {person.name.first} + + {selected && ( + + + + + )} - > - - - - + )} - - )} - - ))} - -
-
+ + ))} + +
+
+ + )}
@@ -253,13 +250,12 @@ export default function App() {
{ - setActiveLocation(location) setQuery('') }} > - {({ open }) => { + {({ open, value }) => { return (
@@ -271,7 +267,7 @@ export default function App() {
From 11c7b7628a3f616b17bb92d5ae6911abba754645 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 15 Jul 2022 22:16:16 +0200 Subject: [PATCH 3/5] improve types, add missing render prop arguments --- .../@headlessui-react/src/components/combobox/combobox.tsx | 1 + .../@headlessui-react/src/components/listbox/listbox.tsx | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 1c3bc91f0..957ea5a0a 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -302,6 +302,7 @@ interface ComboboxRenderPropArg { disabled: boolean activeIndex: number | null activeOption: T | null + value: T } let ComboboxRoot = forwardRefWithAs(function Combobox< diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 8efd81139..cb0b38ea3 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -299,9 +299,10 @@ function stateReducer(state: StateDefinition, action: Actions) { // --- let DEFAULT_LISTBOX_TAG = Fragment -interface ListboxRenderPropArg { +interface ListboxRenderPropArg { open: boolean disabled: boolean + value: TType } let ListboxRoot = forwardRefWithAs(function Listbox< @@ -311,7 +312,7 @@ let ListboxRoot = forwardRefWithAs(function Listbox< >( props: Props< TTag, - ListboxRenderPropArg, + ListboxRenderPropArg, 'value' | 'defaultValue' | 'onChange' | 'by' | 'disabled' | 'horizontal' | 'name' | 'multiple' > & { value?: TType @@ -417,7 +418,7 @@ let ListboxRoot = forwardRefWithAs(function Listbox< listboxState === ListboxStates.Open ) - let slot = useMemo( + let slot = useMemo>( () => ({ open: listboxState === ListboxStates.Open, disabled, value }), [listboxState, disabled, value] ) From 0fd0493f1944c21596ac7ff26d56e541e04771e2 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 29 Jul 2022 13:44:17 +0200 Subject: [PATCH 4/5] add tests for uncontrolled components (React) --- .../src/components/combobox/combobox.test.tsx | 128 +++++++++++++++++ .../src/components/listbox/listbox.test.tsx | 125 +++++++++++++++++ .../radio-group/radio-group.test.tsx | 110 +++++++++++++++ .../src/components/switch/switch.test.tsx | 130 ++++++++++++++++++ 4 files changed, 493 insertions(+) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 4dd5dd594..5ef29f926 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -901,6 +901,134 @@ describe('Rendering', () => { // Verify that the third combobox option is active assertActiveComboboxOption(options[2]) }) + + describe('Uncontrolled', () => { + it('should be possible to use in an uncontrolled way', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + + Trigger + + Alice + Bob + Charlie + + + +
+ ) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Open combobox + await click(getComboboxButton()) + + // Choose alice + await click(getComboboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + + // Open combobox + await click(getComboboxButton()) + + // Choose charlie + await click(getComboboxOptions()[2]) + + // Submit + await click(document.getElementById('submit')) + + // Charlie should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'charlie' }) + }) + + it('should be possible to provide a default value', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + + Trigger + + Alice + Bob + Charlie + + + +
+ ) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'bob' }) + + // Open combobox + await click(getComboboxButton()) + + // Choose alice + await click(getComboboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + }) + + it('should still call the onChange listeners when choosing new values', async () => { + let handleChange = jest.fn() + + render( + + + Trigger + + Alice + Bob + Charlie + + + ) + + // Open combobox + await click(getComboboxButton()) + + // Choose alice + await click(getComboboxOptions()[0]) + + // Open combobox + await click(getComboboxButton()) + + // Choose bob + await click(getComboboxOptions()[1]) + + // Change handler should have been called twice + expect(handleChange).toHaveBeenNthCalledWith(1, 'alice') + expect(handleChange).toHaveBeenNthCalledWith(2, 'bob') + }) + }) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index df432e1d6..d12469e22 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -719,6 +719,131 @@ describe('Rendering', () => { // Verify that the third menu item is active assertActiveListboxOption(options[2]) }) + + describe('Uncontrolled', () => { + it('should be possible to use in an uncontrolled way', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + Trigger + + Alice + Bob + Charlie + + + +
+ ) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Open listbox + await click(getListboxButton()) + + // Choose alice + await click(getListboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + + // Open listbox + await click(getListboxButton()) + + // Choose charlie + await click(getListboxOptions()[2]) + + // Submit + await click(document.getElementById('submit')) + + // Charlie should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'charlie' }) + }) + + it('should be possible to provide a default value', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + Trigger + + Alice + Bob + Charlie + + + +
+ ) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'bob' }) + + // Open listbox + await click(getListboxButton()) + + // Choose alice + await click(getListboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + }) + + it('should still call the onChange listeners when choosing new values', async () => { + let handleChange = jest.fn() + + render( + + Trigger + + Alice + Bob + Charlie + + + ) + + // Open listbox + await click(getListboxButton()) + + // Choose alice + await click(getListboxOptions()[0]) + + // Open listbox + await click(getListboxButton()) + + // Choose bob + await click(getListboxOptions()[1]) + + // Change handler should have been called twice + expect(handleChange).toHaveBeenNthCalledWith(1, 'alice') + expect(handleChange).toHaveBeenNthCalledWith(2, 'bob') + }) + }) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx index 916cf0467..d6a2c84a7 100644 --- a/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx +++ b/packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx @@ -458,6 +458,116 @@ describe('Rendering', () => { }) ) }) + + describe('Uncontrolled', () => { + it( + 'should be possible to use in an uncontrolled way', + suppressConsoleLogs(async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + Alice + Bob + Charlie + + +
+ ) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Choose alice + await click(getRadioGroupOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + + // Choose charlie + await click(getRadioGroupOptions()[2]) + + // Submit + await click(document.getElementById('submit')) + + // Charlie should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'charlie' }) + }) + ) + + it( + 'should be possible to provide a default value', + suppressConsoleLogs(async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + Alice + Bob + Charlie + + +
+ ) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'bob' }) + + // Choose alice + await click(getRadioGroupOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + }) + ) + + it( + 'should still call the onChange listeners when choosing new values', + suppressConsoleLogs(async () => { + let handleChange = jest.fn() + + render( + + Alice + Bob + Charlie + + ) + + // Choose alice + await click(getRadioGroupOptions()[0]) + + // Choose bob + await click(getRadioGroupOptions()[1]) + + // Change handler should have been called twice + expect(handleChange).toHaveBeenNthCalledWith(1, 'alice') + expect(handleChange).toHaveBeenNthCalledWith(2, 'bob') + }) + ) + }) }) describe('Keyboard interactions', () => { diff --git a/packages/@headlessui-react/src/components/switch/switch.test.tsx b/packages/@headlessui-react/src/components/switch/switch.test.tsx index 7b66e9175..811f0832e 100644 --- a/packages/@headlessui-react/src/components/switch/switch.test.tsx +++ b/packages/@headlessui-react/src/components/switch/switch.test.tsx @@ -119,6 +119,136 @@ describe('Rendering', () => { expect(getSwitch()).not.toHaveAttribute('type') }) }) + + describe('Uncontrolled', () => { + it('should be possible to use in an uncontrolled way', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + + + ) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Toggle + await click(getSwitch()) + + // Submit + await click(document.getElementById('submit')) + + // Notifications should be on + expect(handleSubmission).toHaveBeenLastCalledWith({ notifications: 'on' }) + + // Toggle + await click(getSwitch()) + + // Submit + await click(document.getElementById('submit')) + + // Notifications should be off (or in this case, non-existant) + expect(handleSubmission).toHaveBeenLastCalledWith({}) + }) + + it('should be possible to use in an uncontrolled way with a value', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + + + ) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Toggle + await click(getSwitch()) + + // Submit + await click(document.getElementById('submit')) + + // Notifications should be on + expect(handleSubmission).toHaveBeenLastCalledWith({ notifications: 'enabled' }) + + // Toggle + await click(getSwitch()) + + // Submit + await click(document.getElementById('submit')) + + // Notifications should be off (or in this case, non-existant) + expect(handleSubmission).toHaveBeenLastCalledWith({}) + }) + + it('should be possible to provide a default value', async () => { + let handleSubmission = jest.fn() + + render( +
{ + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }} + > + + + + ) + + await click(document.getElementById('submit')) + + // Notifications should be on by default + expect(handleSubmission).toHaveBeenLastCalledWith({ notifications: 'on' }) + + // Toggle + await click(getSwitch()) + + // Submit + await click(document.getElementById('submit')) + + // Notifications should be off (or in this case non-existant) + expect(handleSubmission).toHaveBeenLastCalledWith({}) + }) + + it('should still call the onChange listeners when choosing new values', async () => { + let handleChange = jest.fn() + + render() + + // Toggle + await click(getSwitch()) + + // Toggle + await click(getSwitch()) + + // Toggle + await click(getSwitch()) + + // Change handler should have been called 3 times + expect(handleChange).toHaveBeenNthCalledWith(1, true) + expect(handleChange).toHaveBeenNthCalledWith(2, false) + expect(handleChange).toHaveBeenNthCalledWith(3, true) + }) + }) }) describe('Render composition', () => { From de8474f98032ef8c29353b886e485596ab03c279 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 1 Aug 2022 12:17:12 +0200 Subject: [PATCH 5/5] implement uncontrolled form elements in Vue --- .../src/components/combobox/combobox.test.ts | 139 +++++++++++++++++ .../src/components/combobox/combobox.ts | 37 +++-- .../src/components/listbox/listbox.test.tsx | 134 +++++++++++++++++ .../src/components/listbox/listbox.ts | 25 ++- .../radio-group/radio-group.test.ts | 119 +++++++++++++++ .../src/components/radio-group/radio-group.ts | 20 ++- .../src/components/switch/switch.test.tsx | 142 ++++++++++++++++++ .../src/components/switch/switch.ts | 26 ++-- .../src/hooks/use-controllable.ts | 22 +++ 9 files changed, 628 insertions(+), 36 deletions(-) create mode 100644 packages/@headlessui-vue/src/hooks/use-controllable.ts diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 5454fb89a..043053943 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -982,6 +982,145 @@ describe('Rendering', () => { // Verify that the third combobox option is active assertActiveComboboxOption(options[2]) }) + + describe('Uncontrolled', () => { + it('should be possible to use in an uncontrolled way', async () => { + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + + Trigger + + Alice + Bob + Charlie + + + +
+ `, + setup: () => ({ + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Open combobox + await click(getComboboxButton()) + + // Choose alice + await click(getComboboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + + // Open combobox + await click(getComboboxButton()) + + // Choose charlie + await click(getComboboxOptions()[2]) + + // Submit + await click(document.getElementById('submit')) + + // Charlie should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'charlie' }) + }) + + it('should be possible to provide a default value', async () => { + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + + Trigger + + Alice + Bob + Charlie + + + +
+ `, + setup: () => ({ + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'bob' }) + + // Open combobox + await click(getComboboxButton()) + + // Choose alice + await click(getComboboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + }) + + it('should still call the onChange listeners when choosing new values', async () => { + let handleChange = jest.fn() + + renderTemplate({ + template: html` + + + Trigger + + Alice + Bob + Charlie + + + `, + setup: () => ({ + handleChange, + }), + }) + + // Open combobox + await click(getComboboxButton()) + + // Choose alice + await click(getComboboxOptions()[0]) + + // Open combobox + await click(getComboboxButton()) + + // Choose bob + await click(getComboboxOptions()[1]) + + // Change handler should have been called twice + expect(handleChange).toHaveBeenNthCalledWith(1, 'alice') + expect(handleChange).toHaveBeenNthCalledWith(2, 'bob') + }) + }) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 09e76832c..c31442907 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -34,6 +34,7 @@ import { sortByDomNode } from '../../utils/focus-management' import { useOutsideClick } from '../../hooks/use-outside-click' import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { objectToFormEntries } from '../../utils/form' +import { useControllable } from '../../hooks/use-controllable' function defaultComparator(a: T, z: T): boolean { return a === z @@ -117,7 +118,8 @@ export let Combobox = defineComponent({ as: { type: [Object, String], default: 'template' }, disabled: { type: [Boolean], default: false }, by: { type: [String, Function], default: () => defaultComparator }, - modelValue: { type: [Object, String, Number, Boolean] }, + modelValue: { type: [Object, String, Number, Boolean], default: undefined }, + defaultValue: { type: [Object, String, Number, Boolean], default: undefined }, name: { type: String }, nullable: { type: Boolean, default: false }, multiple: { type: [Boolean], default: false }, @@ -171,9 +173,13 @@ export let Combobox = defineComponent({ } } - let value = computed(() => props.modelValue) let mode = computed(() => (props.multiple ? ValueMode.Multi : ValueMode.Single)) let nullable = computed(() => props.nullable) + let [value, theirOnChange] = useControllable( + computed(() => props.modelValue), + (value: unknown) => emit('update:modelValue', value), + computed(() => props.defaultValue) + ) let api = { comboboxState, @@ -194,7 +200,7 @@ export let Combobox = defineComponent({ disabled: computed(() => props.disabled), options, change(value: unknown) { - emit('update:modelValue', value) + theirOnChange(value as typeof props.modelValue) }, activeOptionIndex: computed(() => { if ( @@ -308,8 +314,7 @@ export let Combobox = defineComponent({ if (!option) return let { dataRef } = option - emit( - 'update:modelValue', + theirOnChange( match(mode.value, { [ValueMode.Single]: () => dataRef.value, [ValueMode.Multi]: () => { @@ -333,8 +338,7 @@ export let Combobox = defineComponent({ if (api.activeOptionIndex.value === null) return let { dataRef, id } = options.value[api.activeOptionIndex.value] - emit( - 'update:modelValue', + theirOnChange( match(mode.value, { [ValueMode.Single]: () => dataRef.value, [ValueMode.Multi]: () => { @@ -440,7 +444,7 @@ export let Combobox = defineComponent({ ) return () => { - let { name, modelValue, disabled, ...theirProps } = props + let { name, disabled, ...theirProps } = props let slot = { open: comboboxState.value === ComboboxStates.Open, disabled, @@ -449,9 +453,9 @@ export let Combobox = defineComponent({ } return h(Fragment, [ - ...(name != null && modelValue != null - ? objectToFormEntries({ [name]: modelValue }).map(([name, value]) => - h( + ...(name != null && value.value != null + ? objectToFormEntries({ [name]: value.value }).map(([name, value]) => { + return h( Hidden, compact({ features: HiddenFeatures.Hidden, @@ -464,12 +468,19 @@ export let Combobox = defineComponent({ value, }) ) - ) + }) : []), render({ theirProps: { ...attrs, - ...omit(theirProps, ['nullable', 'multiple', 'onUpdate:modelValue', 'by']), + ...omit(theirProps, [ + 'modelValue', + 'defaultValue', + 'nullable', + 'multiple', + 'onUpdate:modelValue', + 'by', + ]), }, ourProps: {}, slot, diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index 71d0ee77e..5db764f21 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -806,6 +806,140 @@ describe('Rendering', () => { // Verify that the third listbox option is active assertActiveListboxOption(options[2]) }) + + describe('Uncontrolled', () => { + it('should be possible to use in an uncontrolled way', async () => { + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + Trigger + + Alice + Bob + Charlie + + + +
+ `, + setup: () => ({ + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Open listbox + await click(getListboxButton()) + + // Choose alice + await click(getListboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + + // Open listbox + await click(getListboxButton()) + + // Choose charlie + await click(getListboxOptions()[2]) + + // Submit + await click(document.getElementById('submit')) + + // Charlie should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'charlie' }) + }) + + it('should be possible to provide a default value', async () => { + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + Trigger + + Alice + Bob + Charlie + + + +
+ `, + setup: () => ({ + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'bob' }) + + // Open listbox + await click(getListboxButton()) + + // Choose alice + await click(getListboxOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + }) + + it('should still call the onChange listeners when choosing new values', async () => { + let handleChange = jest.fn() + + renderTemplate({ + template: html` + + Trigger + + Alice + Bob + Charlie + + + `, + setup: () => ({ handleChange }), + }) + + // Open listbox + await click(getListboxButton()) + + // Choose alice + await click(getListboxOptions()[0]) + + // Open listbox + await click(getListboxButton()) + + // Choose bob + await click(getListboxOptions()[1]) + + // Change handler should have been called twice + expect(handleChange).toHaveBeenNthCalledWith(1, 'alice') + expect(handleChange).toHaveBeenNthCalledWith(2, 'bob') + }) + }) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index 4c08c1075..f3dc39769 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -32,6 +32,7 @@ import { FocusableMode, isFocusableElement, sortByDomNode } from '../../utils/fo import { useOutsideClick } from '../../hooks/use-outside-click' import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { objectToFormEntries } from '../../utils/form' +import { useControllable } from '../../hooks/use-controllable' function defaultComparator(a: T, z: T): boolean { return a === z @@ -118,7 +119,8 @@ export let Listbox = defineComponent({ disabled: { type: [Boolean], default: false }, by: { type: [String, Function], default: () => defaultComparator }, horizontal: { type: [Boolean], default: false }, - modelValue: { type: [Object, String, Number, Boolean] }, + modelValue: { type: [Object, String, Number, Boolean], default: undefined }, + defaultValue: { type: [Object, String, Number, Boolean], default: undefined }, name: { type: String, optional: true }, multiple: { type: [Boolean], default: false }, }, @@ -164,8 +166,12 @@ export let Listbox = defineComponent({ } } - let value = computed(() => props.modelValue) let mode = computed(() => (props.multiple ? ValueMode.Multi : ValueMode.Single)) + let [value, theirOnChange] = useControllable( + computed(() => props.modelValue), + (value: unknown) => emit('update:modelValue', value), + computed(() => props.defaultValue) + ) let api = { listboxState, @@ -275,8 +281,7 @@ export let Listbox = defineComponent({ }, select(value: unknown) { if (props.disabled) return - emit( - 'update:modelValue', + theirOnChange( match(mode.value, { [ValueMode.Single]: () => value, [ValueMode.Multi]: () => { @@ -328,8 +333,8 @@ export let Listbox = defineComponent({ let slot = { open: listboxState.value === ListboxStates.Open, disabled } return h(Fragment, [ - ...(name != null && modelValue != null - ? objectToFormEntries({ [name]: modelValue }).map(([name, value]) => + ...(name != null && value.value != null + ? objectToFormEntries({ [name]: value.value }).map(([name, value]) => h( Hidden, compact({ @@ -349,7 +354,13 @@ export let Listbox = defineComponent({ ourProps: {}, theirProps: { ...attrs, - ...omit(theirProps, ['onUpdate:modelValue', 'horizontal', 'multiple', 'by']), + ...omit(theirProps, [ + 'defaultValue', + 'onUpdate:modelValue', + 'horizontal', + 'multiple', + 'by', + ]), }, slot, slots, diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts index 2b5ca7d5b..1aed26d7d 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts @@ -590,6 +590,125 @@ describe('Rendering', () => { }) ) }) + + describe('Uncontrolled', () => { + it( + 'should be possible to use in an uncontrolled way', + suppressConsoleLogs(async () => { + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + Alice + Bob + Charlie + + +
+ `, + setup: () => ({ + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // No values + expect(handleSubmission).toHaveBeenLastCalledWith({}) + + // Choose alice + await click(getRadioGroupOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + + // Choose charlie + await click(getRadioGroupOptions()[2]) + + // Submit + await click(document.getElementById('submit')) + + // Charlie should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'charlie' }) + }) + ) + + it( + 'should be possible to provide a default value', + suppressConsoleLogs(async () => { + let handleSubmission = jest.fn() + + renderTemplate({ + template: html` +
+ + Alice + Bob + Charlie + + +
+ `, + setup: () => ({ + handleSubmit(e: SubmitEvent) { + e.preventDefault() + handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) + }, + }), + }) + + await click(document.getElementById('submit')) + + // Bob is the defaultValue + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'bob' }) + + // Choose alice + await click(getRadioGroupOptions()[0]) + + // Submit + await click(document.getElementById('submit')) + + // Alice should be submitted + expect(handleSubmission).toHaveBeenLastCalledWith({ assignee: 'alice' }) + }) + ) + + it( + 'should still call the onChange listeners when choosing new values', + suppressConsoleLogs(async () => { + let handleChange = jest.fn() + + renderTemplate({ + template: html` + + Alice + Bob + Charlie + + `, + setup: () => ({ handleChange }), + }) + + // Choose alice + await click(getRadioGroupOptions()[0]) + + // Choose bob + await click(getRadioGroupOptions()[1]) + + // Change handler should have been called twice + expect(handleChange).toHaveBeenNthCalledWith(1, 'alice') + expect(handleChange).toHaveBeenNthCalledWith(2, 'bob') + }) + ) + }) }) describe('Keyboard interactions', () => { diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts index c6c05df96..ec0b1f1a8 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts @@ -26,6 +26,7 @@ import { useTreeWalker } from '../../hooks/use-tree-walker' import { Hidden, Features as HiddenFeatures } from '../../internal/hidden' import { attemptSubmit, objectToFormEntries } from '../../utils/form' import { getOwnerDocument } from '../../utils/owner' +import { useControllable } from '../../hooks/use-controllable' function defaultComparator(a: T, z: T): boolean { return a === z @@ -76,7 +77,8 @@ export let RadioGroup = defineComponent({ as: { type: [Object, String], default: 'div' }, disabled: { type: [Boolean], default: false }, by: { type: [String, Function], default: () => defaultComparator }, - modelValue: { type: [Object, String, Number, Boolean] }, + modelValue: { type: [Object, String, Number, Boolean], default: undefined }, + defaultValue: { type: [Object, String, Number, Boolean], default: undefined }, name: { type: String, optional: true }, }, inheritAttrs: false, @@ -88,7 +90,11 @@ export let RadioGroup = defineComponent({ expose({ el: radioGroupRef, $el: radioGroupRef }) - let value = computed(() => props.modelValue) + let [value, theirOnChange] = useControllable( + computed(() => props.modelValue), + (value: unknown) => emit('update:modelValue', value), + computed(() => props.defaultValue) + ) // TODO: Fix type let api: any = { @@ -120,7 +126,7 @@ export let RadioGroup = defineComponent({ api.compare(toRaw(option.propsRef.value), toRaw(nextValue)) )?.propsRef if (nextOption?.disabled) return false - emit('update:modelValue', nextValue) + theirOnChange(nextValue) return true }, registerOption(action: UnwrapRef