Skip to content

Commit 719cac5

Browse files
RobinMalfaitRobinMalfait
authored
Ignore non-option roles (#1081)
* rename `ComboboxState` to `comboboxState` for consistency * ensure all elements between `role: listbox` and `role: option` are marked as `role: none` * add test to demonstrate the `role: none`
1 parent f04d460 commit 719cac5

File tree

4 files changed

+148
-25
lines changed

4 files changed

+148
-25
lines changed

‎packages/@headlessui-react/src/components/combobox/combobox.test.tsx‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,52 @@ describe('Rendering composition', () => {
741741
getComboboxOptions().forEach((option) => assertComboboxOption(option, { tag: 'button' }))
742742
})
743743
)
744+
745+
it(
746+
'should mark all the elements between Combobox.Options and Combobox.Option with role none',
747+
suppressConsoleLogs(async () => {
748+
render(
749+
<Combobox value="test" onChange={console.log}>
750+
<Combobox.Input onChange={NOOP} />
751+
<Combobox.Button />
752+
<div className="outer">
753+
<Combobox.Options>
754+
<div className="inner py-1">
755+
<Combobox.Option value="a">Option A</Combobox.Option>
756+
<Combobox.Option value="b">Option B</Combobox.Option>
757+
</div>
758+
<div className="inner py-1">
759+
<Combobox.Option value="c">Option C</Combobox.Option>
760+
<Combobox.Option value="d">
761+
<div>
762+
<div className="outer">Option D</div>
763+
</div>
764+
</Combobox.Option>
765+
</div>
766+
<div className="inner py-1">
767+
<form className="inner">
768+
<Combobox.Option value="e">Option E</Combobox.Option>
769+
</form>
770+
</div>
771+
</Combobox.Options>
772+
</div>
773+
</Combobox>
774+
)
775+
776+
// Open combobox
777+
await click(getComboboxButton())
778+
779+
expect.hasAssertions()
780+
781+
document.querySelectorAll('.outer').forEach((element) => {
782+
expect(element).not.toHaveAttribute('role', 'none')
783+
})
784+
785+
document.querySelectorAll('.inner').forEach((element) => {
786+
expect(element).toHaveAttribute('role', 'none')
787+
})
788+
})
789+
)
744790
})
745791

746792
describe('Composition', () => {

‎packages/@headlessui-react/src/components/combobox/combobox.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { useWindowEvent } from '../../hooks/use-window-event'
3535
import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed'
3636
import { useResolveButtonType } from '../../hooks/use-resolve-button-type'
3737
import { useLatestValue } from '../../hooks/use-latest-value'
38+
import { useTreeWalker } from '../../hooks/use-tree-walker'
3839

3940
enum ComboboxStates {
4041
Open,
@@ -733,6 +734,19 @@ let Options = forwardRefWithAs(function Options<
733734
return state.comboboxState === ComboboxStates.Open
734735
})()
735736

737+
useTreeWalker({
738+
container: state.optionsRef.current,
739+
enabled: state.comboboxState === ComboboxStates.Open,
740+
accept(node) {
741+
if (node.getAttribute('role') === 'option') return NodeFilter.FILTER_REJECT
742+
if (node.hasAttribute('role')) return NodeFilter.FILTER_SKIP
743+
return NodeFilter.FILTER_ACCEPT
744+
},
745+
walk(node) {
746+
node.setAttribute('role', 'none')
747+
},
748+
})
749+
736750
let labelledby = useComputed(
737751
() => state.labelRef.current?.id ?? state.buttonRef.current?.id,
738752
[state.labelRef.current, state.buttonRef.current]

‎packages/@headlessui-vue/src/components/combobox/combobox.test.tsx‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,55 @@ describe('Rendering composition', () => {
779779
getComboboxOptions().forEach((option) => assertComboboxOption(option, { tag: 'button' }))
780780
})
781781
)
782+
783+
it(
784+
'should mark all the elements between Combobox.Options and Combobox.Option with role none',
785+
suppressConsoleLogs(async () => {
786+
renderTemplate({
787+
template: html`
788+
<Combobox v-model="value">
789+
<ComboboxInput />
790+
<ComboboxButton />
791+
<div class="outer">
792+
<ComboboxOptions>
793+
<div class="inner py-1">
794+
<ComboboxOption value="a">Option A</ComboboxOption>
795+
<ComboboxOption value="b">Option B</ComboboxOption>
796+
</div>
797+
<div class="inner py-1">
798+
<ComboboxOption value="c">Option C</ComboboxOption>
799+
<ComboboxOption value="d">
800+
<div>
801+
<div class="outer">Option D</div>
802+
</div>
803+
</ComboboxOption>
804+
</div>
805+
<div class="inner py-1">
806+
<form class="inner">
807+
<ComboboxOption value="e">Option E</ComboboxOption>
808+
</form>
809+
</div>
810+
</ComboboxOptions>
811+
</div>
812+
</Combobox>
813+
`,
814+
setup: () => ({ value: ref(null) }),
815+
})
816+
817+
// Open combobox
818+
await click(getComboboxButton())
819+
820+
expect.hasAssertions()
821+
822+
document.querySelectorAll('.outer').forEach((element) => {
823+
expect(element).not.toHaveAttribute('role', 'none')
824+
})
825+
826+
document.querySelectorAll('.inner').forEach((element) => {
827+
expect(element).toHaveAttribute('role', 'none')
828+
})
829+
})
830+
)
782831
})
783832

784833
describe('Composition', () => {

‎packages/@headlessui-vue/src/components/combobox/combobox.ts‎

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { useWindowEvent } from '../../hooks/use-window-event'
2525
import { useOpenClosed, State, useOpenClosedProvider } from '../../internal/open-closed'
2626
import { match } from '../../utils/match'
2727
import { useResolveButtonType } from '../../hooks/use-resolve-button-type'
28+
import { useTreeWalker } from '../../hooks/use-tree-walker'
2829

2930
enum ComboboxStates {
3031
Open,
@@ -34,7 +35,7 @@ enum ComboboxStates {
3435
type ComboboxOptionDataRef = Ref<{ disabled: boolean; value: unknown }>
3536
type StateDefinition = {
3637
// State
37-
ComboboxState: Ref<ComboboxStates>
38+
comboboxState: Ref<ComboboxStates>
3839
value: ComputedRef<unknown>
3940

4041
labelRef: Ref<HTMLLabelElement | null>
@@ -84,7 +85,7 @@ export let Combobox = defineComponent({
8485
modelValue: { type: [Object, String, Number, Boolean] },
8586
},
8687
setup(props, { slots, attrs, emit }) {
87-
let ComboboxState = ref<StateDefinition['ComboboxState']['value']>(ComboboxStates.Closed)
88+
let comboboxState = ref<StateDefinition['comboboxState']['value']>(ComboboxStates.Closed)
8889
let labelRef = ref<StateDefinition['labelRef']['value']>(null)
8990
let inputRef = ref<StateDefinition['inputRef']['value']>(null) as StateDefinition['inputRef']
9091
let buttonRef = ref<StateDefinition['buttonRef']['value']>(null) as StateDefinition['buttonRef']
@@ -97,7 +98,7 @@ export let Combobox = defineComponent({
9798
let value = computed(() => props.modelValue)
9899

99100
let api = {
100-
ComboboxState,
101+
comboboxState,
101102
value,
102103
inputRef,
103104
labelRef,
@@ -109,18 +110,18 @@ export let Combobox = defineComponent({
109110
inputPropsRef: ref<{ displayValue?: (item: unknown) => string }>({ displayValue: undefined }),
110111
closeCombobox() {
111112
if (props.disabled) return
112-
if (ComboboxState.value === ComboboxStates.Closed) return
113-
ComboboxState.value = ComboboxStates.Closed
113+
if (comboboxState.value === ComboboxStates.Closed) return
114+
comboboxState.value = ComboboxStates.Closed
114115
activeOptionIndex.value = null
115116
},
116117
openCombobox() {
117118
if (props.disabled) return
118-
if (ComboboxState.value === ComboboxStates.Open) return
119-
ComboboxState.value = ComboboxStates.Open
119+
if (comboboxState.value === ComboboxStates.Open) return
120+
comboboxState.value = ComboboxStates.Open
120121
},
121122
goToOption(focus: Focus, id?: string) {
122123
if (props.disabled) return
123-
if (ComboboxState.value === ComboboxStates.Closed) return
124+
if (comboboxState.value === ComboboxStates.Closed) return
124125

125126
let nextActiveOptionIndex = calculateActiveIndex(
126127
focus === Focus.Specific
@@ -209,7 +210,7 @@ export let Combobox = defineComponent({
209210
let target = event.target as HTMLElement
210211
let active = document.activeElement
211212

212-
if (ComboboxState.value !== ComboboxStates.Open) return
213+
if (comboboxState.value !== ComboboxStates.Open) return
213214

214215
if (dom(inputRef)?.contains(target)) return
215216
if (dom(buttonRef)?.contains(target)) return
@@ -229,15 +230,15 @@ export let Combobox = defineComponent({
229230
provide(ComboboxContext, api)
230231
useOpenClosedProvider(
231232
computed(() =>
232-
match(ComboboxState.value, {
233+
match(comboboxState.value, {
233234
[ComboboxStates.Open]: State.Open,
234235
[ComboboxStates.Closed]: State.Closed,
235236
})
236237
)
237238
)
238239

239240
return () => {
240-
let slot = { open: ComboboxState.value === ComboboxStates.Open, disabled: props.disabled }
241+
let slot = { open: comboboxState.value === ComboboxStates.Open, disabled: props.disabled }
241242
return render({
242243
props: omit(props, ['modelValue', 'onUpdate:modelValue', 'disabled', 'horizontal']),
243244
slot,
@@ -264,7 +265,7 @@ export let ComboboxLabel = defineComponent({
264265

265266
return () => {
266267
let slot = {
267-
open: api.ComboboxState.value === ComboboxStates.Open,
268+
open: api.comboboxState.value === ComboboxStates.Open,
268269
disabled: api.disabled.value,
269270
}
270271

@@ -294,7 +295,7 @@ export let ComboboxButton = defineComponent({
294295

295296
function handleClick(event: MouseEvent) {
296297
if (api.disabled.value) return
297-
if (api.ComboboxState.value === ComboboxStates.Open) {
298+
if (api.comboboxState.value === ComboboxStates.Open) {
298299
api.closeCombobox()
299300
} else {
300301
event.preventDefault()
@@ -311,7 +312,7 @@ export let ComboboxButton = defineComponent({
311312
case Keys.ArrowDown:
312313
event.preventDefault()
313314
event.stopPropagation()
314-
if (api.ComboboxState.value === ComboboxStates.Closed) {
315+
if (api.comboboxState.value === ComboboxStates.Closed) {
315316
api.openCombobox()
316317
// TODO: We can't do this outside next frame because the options aren't rendered yet
317318
// But doing this in next frame results in a flicker because the dom mutations are async here
@@ -332,7 +333,7 @@ export let ComboboxButton = defineComponent({
332333
case Keys.ArrowUp:
333334
event.preventDefault()
334335
event.stopPropagation()
335-
if (api.ComboboxState.value === ComboboxStates.Closed) {
336+
if (api.comboboxState.value === ComboboxStates.Closed) {
336337
api.openCombobox()
337338
nextTick(() => {
338339
if (!api.value.value) {
@@ -359,7 +360,7 @@ export let ComboboxButton = defineComponent({
359360

360361
return () => {
361362
let slot = {
362-
open: api.ComboboxState.value === ComboboxStates.Open,
363+
open: api.comboboxState.value === ComboboxStates.Open,
363364
disabled: api.disabled.value,
364365
}
365366
let propsWeControl = {
@@ -371,7 +372,7 @@ export let ComboboxButton = defineComponent({
371372
'aria-controls': dom(api.optionsRef)?.id,
372373
'aria-expanded': api.disabled.value
373374
? undefined
374-
: api.ComboboxState.value === ComboboxStates.Open,
375+
: api.comboboxState.value === ComboboxStates.Open,
375376
'aria-labelledby': api.labelRef.value ? [dom(api.labelRef)?.id, id].join(' ') : undefined,
376377
disabled: api.disabled.value === true ? true : undefined,
377378
onKeydown: handleKeydown,
@@ -422,7 +423,7 @@ export let ComboboxInput = defineComponent({
422423
case Keys.ArrowDown:
423424
event.preventDefault()
424425
event.stopPropagation()
425-
return match(api.ComboboxState.value, {
426+
return match(api.comboboxState.value, {
426427
[ComboboxStates.Open]: () => api.goToOption(Focus.Next),
427428
[ComboboxStates.Closed]: () => {
428429
api.openCombobox()
@@ -437,7 +438,7 @@ export let ComboboxInput = defineComponent({
437438
case Keys.ArrowUp:
438439
event.preventDefault()
439440
event.stopPropagation()
440-
return match(api.ComboboxState.value, {
441+
return match(api.comboboxState.value, {
441442
[ComboboxStates.Open]: () => api.goToOption(Focus.Previous),
442443
[ComboboxStates.Closed]: () => {
443444
api.openCombobox()
@@ -480,7 +481,7 @@ export let ComboboxInput = defineComponent({
480481
}
481482

482483
return () => {
483-
let slot = { open: api.ComboboxState.value === ComboboxStates.Open }
484+
let slot = { open: api.comboboxState.value === ComboboxStates.Open }
484485
let propsWeControl = {
485486
'aria-activedescendant':
486487
api.activeOptionIndex.value === null
@@ -527,11 +528,24 @@ export let ComboboxOptions = defineComponent({
527528
return usesOpenClosedState.value === State.Open
528529
}
529530

530-
return api.ComboboxState.value === ComboboxStates.Open
531+
return api.comboboxState.value === ComboboxStates.Open
532+
})
533+
534+
useTreeWalker({
535+
container: computed(() => dom(api.optionsRef)),
536+
enabled: computed(() => api.comboboxState.value === ComboboxStates.Open),
537+
accept(node) {
538+
if (node.getAttribute('role') === 'option') return NodeFilter.FILTER_REJECT
539+
if (node.hasAttribute('role')) return NodeFilter.FILTER_SKIP
540+
return NodeFilter.FILTER_ACCEPT
541+
},
542+
walk(node) {
543+
node.setAttribute('role', 'none')
544+
},
531545
})
532546

533547
return () => {
534-
let slot = { open: api.ComboboxState.value === ComboboxStates.Open }
548+
let slot = { open: api.comboboxState.value === ComboboxStates.Open }
535549
let propsWeControl = {
536550
'aria-activedescendant':
537551
api.activeOptionIndex.value === null
@@ -586,9 +600,9 @@ export let ComboboxOption = defineComponent({
586600

587601
onMounted(() => {
588602
watch(
589-
[api.ComboboxState, selected],
603+
[api.comboboxState, selected],
590604
() => {
591-
if (api.ComboboxState.value !== ComboboxStates.Open) return
605+
if (api.comboboxState.value !== ComboboxStates.Open) return
592606
if (!selected.value) return
593607
api.goToOption(Focus.Specific, id)
594608
},
@@ -597,7 +611,7 @@ export let ComboboxOption = defineComponent({
597611
})
598612

599613
watchEffect(() => {
600-
if (api.ComboboxState.value !== ComboboxStates.Open) return
614+
if (api.comboboxState.value !== ComboboxStates.Open) return
601615
if (!active.value) return
602616
nextTick(() => document.getElementById(id)?.scrollIntoView?.({ block: 'nearest' }))
603617
})

0 commit comments

Comments
 (0)