From 80f4039aec8122c40ad0324761bdf338959e5816 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 12 May 2021 12:52:14 +0200 Subject: [PATCH 1/4] only destructure from props inside render --- .../src/components/listbox/listbox.vue | 8 ++- .../src/components/listbox/listbox.ts | 58 +++++++++++-------- .../src/components/menu/menu.ts | 14 ++--- .../src/components/popover/popover.ts | 4 +- packages/@headlessui-vue/src/utils/render.ts | 2 +- 5 files changed, 48 insertions(+), 38 deletions(-) diff --git a/packages/@headlessui-vue/examples/src/components/listbox/listbox.vue b/packages/@headlessui-vue/examples/src/components/listbox/listbox.vue index 3646e5ccf7..a6c73af43d 100644 --- a/packages/@headlessui-vue/examples/src/components/listbox/listbox.vue +++ b/packages/@headlessui-vue/examples/src/components/listbox/listbox.vue @@ -40,6 +40,7 @@ :key="person.id" :value="person" :className="resolveListboxOptionClassName" + :disabled="person.disabled" v-slot="{ active, selected }" > (ListboxStates.Closed) let labelRef = ref(null) let buttonRef = ref(null) @@ -100,23 +99,23 @@ export let Listbox = defineComponent({ labelRef, buttonRef, optionsRef, - disabled, + disabled: computed(() => props.disabled), options, searchQuery, activeOptionIndex, closeListbox() { - if (disabled) return + if (props.disabled) return if (listboxState.value === ListboxStates.Closed) return listboxState.value = ListboxStates.Closed activeOptionIndex.value = null }, openListbox() { - if (disabled) return + if (props.disabled) return if (listboxState.value === ListboxStates.Open) return listboxState.value = ListboxStates.Open }, goToOption(focus: Focus, id?: string) { - if (disabled) return + if (props.disabled) return if (listboxState.value === ListboxStates.Closed) return let nextActiveOptionIndex = calculateActiveIndex( @@ -136,7 +135,7 @@ export let Listbox = defineComponent({ activeOptionIndex.value = nextActiveOptionIndex }, search(value: string) { - if (disabled) return + if (props.disabled) return if (listboxState.value === ListboxStates.Closed) return searchQuery.value += value.toLowerCase() @@ -150,7 +149,7 @@ export let Listbox = defineComponent({ activeOptionIndex.value = match }, clearSearch() { - if (disabled) return + if (props.disabled) return if (listboxState.value === ListboxStates.Closed) return if (searchQuery.value === '') return @@ -177,7 +176,7 @@ export let Listbox = defineComponent({ })() }, select(value: unknown) { - if (disabled) return + if (props.disabled) return emit('update:modelValue', value) }, } @@ -206,8 +205,14 @@ export let Listbox = defineComponent({ ) return () => { - let slot = { open: listboxState.value === ListboxStates.Open, disabled } - return render({ props: passThroughProps, slot, slots, attrs, name: 'Listbox' }) + let slot = { open: listboxState.value === ListboxStates.Open, disabled: props.disabled } + return render({ + props: omit(props, ['modelValue', 'onUpdate:modelValue', 'disabled']), + slot, + slots, + attrs, + name: 'Listbox', + }) } }, }) @@ -220,7 +225,7 @@ export let ListboxLabel = defineComponent({ render() { let api = useListboxContext('ListboxLabel') - let slot = { open: api.listboxState.value === ListboxStates.Open, disabled: api.disabled } + let slot = { open: api.listboxState.value === ListboxStates.Open, disabled: api.disabled.value } let propsWeControl = { id: this.id, ref: 'el', onClick: this.handleClick } return render({ @@ -255,7 +260,7 @@ export let ListboxButton = defineComponent({ render() { let api = useListboxContext('ListboxButton') - let slot = { open: api.listboxState.value === ListboxStates.Open, disabled: api.disabled } + let slot = { open: api.listboxState.value === ListboxStates.Open, disabled: api.disabled.value } let propsWeControl = { ref: 'el', id: this.id, @@ -266,7 +271,7 @@ export let ListboxButton = defineComponent({ 'aria-labelledby': api.labelRef.value ? [dom(api.labelRef)?.id, this.id].join(' ') : undefined, - disabled: api.disabled, + disabled: api.disabled.value, onKeydown: this.handleKeyDown, onKeyup: this.handleKeyUp, onClick: this.handleClick, @@ -322,7 +327,7 @@ export let ListboxButton = defineComponent({ } function handleClick(event: MouseEvent) { - if (api.disabled) return + if (api.disabled.value) return if (api.listboxState.value === ListboxStates.Open) { api.closeListbox() nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true })) @@ -472,7 +477,6 @@ export let ListboxOption = defineComponent({ setup(props, { slots, attrs }) { let api = useListboxContext('ListboxOption') let id = `headlessui-listbox-option-${useId()}` - let { disabled, class: defaultClass, className = defaultClass, value } = props let active = computed(() => { return api.activeOptionIndex.value !== null @@ -480,9 +484,13 @@ export let ListboxOption = defineComponent({ : false }) - let selected = computed(() => toRaw(api.value.value) === toRaw(value)) + let selected = computed(() => toRaw(api.value.value) === toRaw(props.value)) - let dataRef = ref({ disabled, value, textValue: '' }) + let dataRef = ref({ + disabled: props.disabled, + value: props.value, + textValue: '', + }) onMounted(() => { let textValue = document .getElementById(id) @@ -514,38 +522,40 @@ export let ListboxOption = defineComponent({ }) function handleClick(event: MouseEvent) { - if (disabled) return event.preventDefault() - api.select(value) + if (props.disabled) return event.preventDefault() + api.select(props.value) api.closeListbox() nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true })) } function handleFocus() { - if (disabled) return api.goToOption(Focus.Nothing) + if (props.disabled) return api.goToOption(Focus.Nothing) api.goToOption(Focus.Specific, id) } function handleMove() { - if (disabled) return + if (props.disabled) return if (active.value) return api.goToOption(Focus.Specific, id) } function handleLeave() { - if (disabled) return + if (props.disabled) return if (!active.value) return api.goToOption(Focus.Nothing) } return () => { + let { disabled, class: defaultClass, className = defaultClass } = props let slot = { active: active.value, selected: selected.value, disabled } let propsWeControl = { id, role: 'option', - tabIndex: -1, + tabIndex: disabled === true ? undefined : -1, class: resolvePropValue(className, slot), 'aria-disabled': disabled === true ? true : undefined, 'aria-selected': selected.value === true ? selected.value : undefined, + disabled: undefined, // Never forward the `disabled` prop onClick: handleClick, onFocus: handleFocus, onPointermove: handleMove, diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts index ddb3b15313..dc4a625867 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.ts +++ b/packages/@headlessui-vue/src/components/menu/menu.ts @@ -418,7 +418,6 @@ export let MenuItem = defineComponent({ setup(props, { slots, attrs }) { let api = useMenuContext('MenuItem') let id = `headlessui-menu-item-${useId()}` - let { disabled, class: defaultClass, className = defaultClass } = props let active = computed(() => { return api.activeItemIndex.value !== null @@ -426,7 +425,7 @@ export let MenuItem = defineComponent({ : false }) - let dataRef = ref({ disabled, textValue: '' }) + let dataRef = ref({ disabled: props.disabled, textValue: '' }) onMounted(() => { let textValue = document .getElementById(id) @@ -445,34 +444,35 @@ export let MenuItem = defineComponent({ }) function handleClick(event: MouseEvent) { - if (disabled) return event.preventDefault() + if (props.disabled) return event.preventDefault() api.closeMenu() nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true })) } function handleFocus() { - if (disabled) return api.goToItem(Focus.Nothing) + if (props.disabled) return api.goToItem(Focus.Nothing) api.goToItem(Focus.Specific, id) } function handleMove() { - if (disabled) return + if (props.disabled) return if (active.value) return api.goToItem(Focus.Specific, id) } function handleLeave() { - if (disabled) return + if (props.disabled) return if (!active.value) return api.goToItem(Focus.Nothing) } return () => { + let { disabled, class: defaultClass, className = defaultClass } = props let slot = { active: active.value, disabled } let propsWeControl = { id, role: 'menuitem', - tabIndex: -1, + tabIndex: disabled === true ? undefined : -1, class: resolvePropValue(className, slot), 'aria-disabled': disabled === true ? true : undefined, onClick: handleClick, diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index 458e3364d7..96620be25a 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -87,8 +87,6 @@ export let Popover = defineComponent({ as: { type: [Object, String], default: 'div' }, }, setup(props, { slots, attrs }) { - let { ...passThroughProps } = props - let buttonId = `headlessui-popover-button-${useId()}` let panelId = `headlessui-popover-panel-${useId()}` @@ -178,7 +176,7 @@ export let Popover = defineComponent({ return () => { let slot = { open: popoverState.value === PopoverStates.Open } - return render({ props: passThroughProps, slot, slots, attrs, name: 'Popover' }) + return render({ props, slot, slots, attrs, name: 'Popover' }) } }, }) diff --git a/packages/@headlessui-vue/src/utils/render.ts b/packages/@headlessui-vue/src/utils/render.ts index 87882bc8d8..27c3b77428 100644 --- a/packages/@headlessui-vue/src/utils/render.ts +++ b/packages/@headlessui-vue/src/utils/render.ts @@ -125,7 +125,7 @@ function _render({ return h(as, passThroughProps, children) } -function omit>(object: T, keysToOmit: string[] = []) { +export function omit>(object: T, keysToOmit: string[] = []) { let clone = Object.assign({}, object) for (let key of keysToOmit) { if (key in clone) delete clone[key] From 69a04900cde59dcce8575fec99ae5e153248017f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 12 May 2021 12:59:02 +0200 Subject: [PATCH 2/4] conditionally ensure that tabindex -1 exists --- .../src/test-utils/accessibility-assertions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts index 828d460101..0936094bbc 100644 --- a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts @@ -193,7 +193,7 @@ export function assertMenuItem( // Check that we have the correct values for certain attributes expect(item).toHaveAttribute('role', 'menuitem') - expect(item).toHaveAttribute('tabindex', '-1') + if (!item.getAttribute('aria-disabled')) expect(item).toHaveAttribute('tabindex', '-1') // Ensure menu button has the following attributes if (options) { @@ -483,7 +483,7 @@ export function assertListboxOption( // Check that we have the correct values for certain attributes expect(item).toHaveAttribute('role', 'option') - expect(item).toHaveAttribute('tabindex', '-1') + if (!item.getAttribute('aria-disabled')) expect(item).toHaveAttribute('tabindex', '-1') // Ensure listbox button has the following attributes if (!options) return From 21157da4c1f6eefe95cdf658739a595020d33747 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 12 May 2021 13:57:04 +0200 Subject: [PATCH 3/4] reflect `disabled` prop in React as well --- packages/@headlessui-react/src/components/listbox/listbox.tsx | 3 ++- packages/@headlessui-react/src/components/menu/menu.tsx | 3 ++- .../src/test-utils/accessibility-assertions.ts | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 1d2ad6c290..c886a3fbfe 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -660,9 +660,10 @@ function Option< let propsWeControl = { id, role: 'option', - tabIndex: -1, + tabIndex: disabled === true ? undefined : -1, 'aria-disabled': disabled === true ? true : undefined, 'aria-selected': selected === true ? true : undefined, + disabled: undefined, // Never forward the `disabled` prop onClick: handleClick, onFocus: handleFocus, onPointerMove: handleMove, diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 5440e99b23..9283882099 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -560,8 +560,9 @@ function Item( let propsWeControl = { id, role: 'menuitem', - tabIndex: -1, + tabIndex: disabled === true ? undefined : -1, 'aria-disabled': disabled === true ? true : undefined, + disabled: undefined, // Never forward the `disabled` prop onClick: handleClick, onFocus: handleFocus, onPointerMove: handleMove, diff --git a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts index 828d460101..0936094bbc 100644 --- a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts +++ b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts @@ -193,7 +193,7 @@ export function assertMenuItem( // Check that we have the correct values for certain attributes expect(item).toHaveAttribute('role', 'menuitem') - expect(item).toHaveAttribute('tabindex', '-1') + if (!item.getAttribute('aria-disabled')) expect(item).toHaveAttribute('tabindex', '-1') // Ensure menu button has the following attributes if (options) { @@ -483,7 +483,7 @@ export function assertListboxOption( // Check that we have the correct values for certain attributes expect(item).toHaveAttribute('role', 'option') - expect(item).toHaveAttribute('tabindex', '-1') + if (!item.getAttribute('aria-disabled')) expect(item).toHaveAttribute('tabindex', '-1') // Ensure listbox button has the following attributes if (!options) return From fc8ce9045fb2f7bccbb6f7b6a182340d402c79b0 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 12 May 2021 14:07:49 +0200 Subject: [PATCH 4/4] update changelog --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5f9eac530..f720649a1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,12 +11,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure that you can use `Transition.Child` when using implicit Transitions ([#503](https://github.com/tailwindlabs/headlessui/pull/503)) +### Fixes + +- Improve `disabled` and `tabindex` prop handling ([#512](https://github.com/tailwindlabs/headlessui/pull/512)) + ## [Unreleased - Vue] ### Added - Ensure that you can use `TransitionChild` when using implicit Transitions ([#503](https://github.com/tailwindlabs/headlessui/pull/503)) +### Fixes + +- Improve `disabled` and `tabindex` prop handling ([#512](https://github.com/tailwindlabs/headlessui/pull/512)) +- Improve reactivity when destructuring from props ([#512](https://github.com/tailwindlabs/headlessui/pull/512)) + ## [@headlessui/react@v1.2.0] - 2021-05-10 ### Added