Skip to content

Commit

Permalink
Ensure Combobox.Label is properly linked when rendered after `Combo…
Browse files Browse the repository at this point in the history
…box.Button` and `Combobox.Input` components (#1838)

* ensure `Combbox.Label` is properly linked when rendered after other components

Even when rendered after the Combobox.Input / Combobox.Button

* update changelog
  • Loading branch information
RobinMalfait committed Sep 8, 2022
1 parent b296b73 commit 46a7ab6
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 8 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
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
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
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
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

0 comments on commit 46a7ab6

Please sign in to comment.