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

Ensure Combobox.Label is properly linked when rendered after Combobox.Button and Combobox.Input components #1838

Merged
merged 2 commits into from
Sep 8, 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
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve iOS scroll locking ([#1830](https://github.com/tailwindlabs/headlessui/pull/1830))
- Add `<fieldset disabled>` check to radio group options in React ([#1835](https://github.com/tailwindlabs/headlessui/pull/1835))
- Ensure `Tab` order stays consistent, and the currently active `Tab` stays active ([#1837](https://github.com/tailwindlabs/headlessui/pull/1837))
- Ensure `Combobox.Label` is properly linked when rendered after `Combobox.Button` and `Combobox.Input` components ([#1838](https://github.com/tailwindlabs/headlessui/pull/1838))

## [1.7.0] - 2022-09-06

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,22 @@ describe('Rendering', () => {
})
)

it(
'should be possible to link Input/Button and Label if Label is rendered last',
suppressConsoleLogs(async () => {
render(
<Combobox value="Test" onChange={console.log}>
<Combobox.Input onChange={NOOP} />
<Combobox.Button />
<Combobox.Label>Label</Combobox.Label>
</Combobox>
)

assertComboboxLabelLinkedWithCombobox()
assertComboboxButtonLinkedWithComboboxLabel()
})
)

it(
'should be possible to render a Combobox.Label using a render prop and an `as` prop',
suppressConsoleLogs(async () => {
Expand Down
37 changes: 29 additions & 8 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type ComboboxOptionDataRef<T> = MutableRefObject<{

interface StateDefinition<T> {
dataRef: MutableRefObject<_Data>
labelId: string | null

comboboxState: ComboboxState

Expand All @@ -83,6 +84,8 @@ enum ActionTypes {

RegisterOption,
UnregisterOption,

RegisterLabel,
}

function adjustOrderedState<T>(
Expand Down Expand Up @@ -124,6 +127,7 @@ type Actions<T> =
trigger?: ActivationTrigger
}
| { type: ActionTypes.RegisterOption; id: string; dataRef: ComboboxOptionDataRef<T> }
| { type: ActionTypes.RegisterLabel; id: string | null }
| { type: ActionTypes.UnregisterOption; id: string }

let reducers: {
Expand Down Expand Up @@ -227,12 +231,19 @@ let reducers: {
activationTrigger: ActivationTrigger.Other,
}
},
[ActionTypes.RegisterLabel]: (state, action) => {
return {
...state,
labelId: action.id,
}
},
}

let ComboboxActionsContext = createContext<{
openCombobox(): void
closeCombobox(): void
registerOption(id: string, dataRef: ComboboxOptionDataRef<unknown>): () => void
registerLabel(id: string): () => void
goToOption(focus: Focus.Specific, id: string, trigger?: ActivationTrigger): void
goToOption(focus: Focus, id?: string, trigger?: ActivationTrigger): void
selectOption(id: string): void
Expand Down Expand Up @@ -402,6 +413,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
options: [],
activeOptionIndex: null,
activationTrigger: ActivationTrigger.Other,
labelId: null,
} as StateDefinition<TValue>)

let defaultToFirstOption = useRef(false)
Expand Down Expand Up @@ -536,6 +548,11 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
return () => dispatch({ type: ActionTypes.UnregisterOption, id })
})

let registerLabel = useEvent((id) => {
dispatch({ type: ActionTypes.RegisterLabel, id })
return () => dispatch({ type: ActionTypes.RegisterLabel, id: null })
})

let onChange = useEvent((value: unknown) => {
return match(data.mode, {
[ValueMode.Single]() {
Expand All @@ -560,6 +577,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
() => ({
onChange,
registerOption,
registerLabel,
goToOption,
closeCombobox,
openCombobox,
Expand Down Expand Up @@ -775,9 +793,9 @@ let Input = forwardRefWithAs(function Input<
// TODO: Verify this. The spec says that, for the input/combobox, the label is the labelling element when present
// Otherwise it's the ID of the non-label element
let labelledby = useComputed(() => {
if (!data.labelRef.current) return undefined
return [data.labelRef.current.id].join(' ')
}, [data.labelRef.current])
if (!data.labelId) return undefined
return [data.labelId].join(' ')
}, [data.labelId])

let slot = useMemo<InputRenderPropArg>(
() => ({ open: data.comboboxState === ComboboxState.Open, disabled: data.disabled }),
Expand Down Expand Up @@ -892,9 +910,9 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
})

let labelledby = useComputed(() => {
if (!data.labelRef.current) return undefined
return [data.labelRef.current.id, id].join(' ')
}, [data.labelRef.current, id])
if (!data.labelId) return undefined
return [data.labelId, id].join(' ')
}, [data.labelId, id])

let slot = useMemo<ButtonRenderPropArg>(
() => ({
Expand Down Expand Up @@ -943,8 +961,11 @@ let Label = forwardRefWithAs(function Label<TTag extends ElementType = typeof DE
) {
let data = useData('Combobox.Label')
let id = `headlessui-combobox-label-${useId()}`
let actions = useActions('Combobox.Label')
let labelRef = useSyncRefs(data.labelRef, ref)

useIsoMorphicEffect(() => actions.registerLabel(id), [id])

let handleClick = useEvent(() => data.inputRef.current?.focus({ preventScroll: true }))

let slot = useMemo<LabelRenderPropArg>(
Expand Down Expand Up @@ -1027,8 +1048,8 @@ let Options = forwardRefWithAs(function Options<
})

let labelledby = useComputed(
() => data.labelRef.current?.id ?? data.buttonRef.current?.id,
[data.labelRef.current, data.buttonRef.current]
() => data.labelId ?? data.buttonRef.current?.id,
[data.labelId, data.buttonRef.current]
)

let slot = useMemo<OptionsRenderPropArg>(
Expand Down
21 changes: 21 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,27 @@ describe('Rendering', () => {
})
)

it(
'should be possible to link Input/Button and Label if Label is rendered last',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<Combobox v-model="value">
<ComboboxInput />
<ComboboxButton />
<ComboboxLabel>Label</ComboboxLabel>
</Combobox>
`,
setup: () => ({ value: ref(null) }),
})

await new Promise<void>(nextTick)

assertComboboxLabelLinkedWithCombobox()
assertComboboxButtonLinkedWithComboboxLabel()
})
)

it(
'should be possible to render a ComboboxLabel using a render prop and an `as` prop',
suppressConsoleLogs(async () => {
Expand Down