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

Make form components uncontrollable #1683

Merged
merged 5 commits into from
Aug 1, 2022
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
128 changes: 128 additions & 0 deletions packages/@headlessui-react/src/components/combobox/combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<form
onSubmit={(e) => {
e.preventDefault()
handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
}}
>
<Combobox name="assignee">
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="alice">Alice</Combobox.Option>
<Combobox.Option value="bob">Bob</Combobox.Option>
<Combobox.Option value="charlie">Charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
<button id="submit">submit</button>
</form>
)

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(
<form
onSubmit={(e) => {
e.preventDefault()
handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
}}
>
<Combobox name="assignee" defaultValue="bob">
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="alice">Alice</Combobox.Option>
<Combobox.Option value="bob">Bob</Combobox.Option>
<Combobox.Option value="charlie">Charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
<button id="submit">submit</button>
</form>
)

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(
<Combobox name="assignee" onChange={handleChange}>
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="alice">Alice</Combobox.Option>
<Combobox.Option value="bob">Bob</Combobox.Option>
<Combobox.Option value="charlie">Charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
)

// 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', () => {
Expand Down
22 changes: 14 additions & 8 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -301,6 +302,7 @@ interface ComboboxRenderPropArg<T> {
disabled: boolean
activeIndex: number | null
activeOption: T | null
value: T
}

let ComboboxRoot = forwardRefWithAs(function Combobox<
Expand All @@ -311,10 +313,11 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
props: Props<
TTag,
ComboboxRenderPropArg<TType>,
'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
Expand All @@ -325,16 +328,18 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
ref: Ref<TTag>
) {
let {
value: controlledValue,
defaultValue,
onChange: controlledOnChange,
name,
value,
onChange: theirOnChange,
by = (a, z) => a === z,
disabled = false,
__demoMode = false,
nullable = false,
multiple = false,
...theirProps
} = props
let [value, theirOnChange] = useControllable(controlledValue, controlledOnChange, defaultValue)

let [state, dispatch] = useReducer(stateReducer, {
dataRef: createRef(),
Expand Down Expand Up @@ -430,8 +435,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(() => {
Expand Down Expand Up @@ -495,7 +501,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()
Expand All @@ -507,7 +513,7 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
copy.splice(idx, 1)
}

return theirOnChange(copy as unknown as TType)
return theirOnChange?.(copy as unknown as TType)
},
})
})
Expand Down
125 changes: 125 additions & 0 deletions packages/@headlessui-react/src/components/listbox/listbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<form
onSubmit={(e) => {
e.preventDefault()
handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
}}
>
<Listbox name="assignee">
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="alice">Alice</Listbox.Option>
<Listbox.Option value="bob">Bob</Listbox.Option>
<Listbox.Option value="charlie">Charlie</Listbox.Option>
</Listbox.Options>
</Listbox>
<button id="submit">submit</button>
</form>
)

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(
<form
onSubmit={(e) => {
e.preventDefault()
handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
}}
>
<Listbox name="assignee" defaultValue="bob">
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="alice">Alice</Listbox.Option>
<Listbox.Option value="bob">Bob</Listbox.Option>
<Listbox.Option value="charlie">Charlie</Listbox.Option>
</Listbox.Options>
</Listbox>
<button id="submit">submit</button>
</form>
)

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(
<Listbox name="assignee" onChange={handleChange}>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="alice">Alice</Listbox.Option>
<Listbox.Option value="bob">Bob</Listbox.Option>
<Listbox.Option value="charlie">Charlie</Listbox.Option>
</Listbox.Options>
</Listbox>
)

// 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', () => {
Expand Down
Loading