Improve Vue reactivity & disabled prop

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/[email protected]] - 2021-05-10
 
 ### Added 

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,

packages/@headlessui-react/src/components/menu/menu.tsx

@@ -560,8 +560,9 @@ function Item<TTag extends ElementType = typeof DEFAULT_ITEM_TAG>(
   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,

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

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 }"
                 >
                   <span
@@ -98,7 +99,7 @@ export default {
       { id: 2, name: 'Arlene Mccoy' },
       { id: 3, name: 'Devon Webb' },
       { id: 4, name: 'Tom Cook' },
-      { id: 5, name: 'Tanya Fox' },
+      { id: 5, name: 'Tanya Fox', disabled: true },
       { id: 6, name: 'Hellen Schmidt' },
       { id: 7, name: 'Caroline Schultz' },
       { id: 8, name: 'Mason Heaney' },
@@ -112,10 +113,11 @@ export default {
       people,
       active,
       classNames,
-      resolveListboxOptionClassName({ active }) {
+      resolveListboxOptionClassName({ active, disabled }) {
         return classNames(
           'relative py-2 pl-3 cursor-default select-none pr-9 focus:outline-none',
-          active ? 'text-white bg-indigo-600' : 'text-gray-900'
+          active ? 'text-white bg-indigo-600' : 'text-gray-900',
+          disabled && 'bg-gray-50 text-gray-300'
         )
       },
     }

packages/@headlessui-vue/src/components/listbox/listbox.ts

@@ -15,7 +15,7 @@ import {
   watch,
 } from 'vue'
 
-import { Features, render } from '../../utils/render'
+import { Features, render, omit } from '../../utils/render'
 import { useId } from '../../hooks/use-id'
 import { Keys } from '../../keyboard'
 import { calculateActiveIndex, Focus } from '../../utils/calculate-active-index'
@@ -83,7 +83,6 @@ export let Listbox = defineComponent({
     modelValue: { type: [Object, String, Number, Boolean] },
   },
   setup(props, { slots, attrs, emit }) {
-    let { modelValue, disabled, ...passThroughProps } = props
     let listboxState = ref<StateDefinition['listboxState']['value']>(ListboxStates.Closed)
     let labelRef = ref<StateDefinition['labelRef']['value']>(null)
     let buttonRef = ref<StateDefinition['buttonRef']['value']>(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,17 +477,20 @@ 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
         ? api.options.value[api.activeOptionIndex.value].id === id
         : false
     })
 
-    let selected = computed(() => toRaw(api.value.value) === toRaw(value))
+    let selected = computed(() => toRaw(api.value.value) === toRaw(props.value))
 
-    let dataRef = ref<ListboxOptionDataRef['value']>({ disabled, value, textValue: '' })
+    let dataRef = ref<ListboxOptionDataRef['value']>({
+      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,

packages/@headlessui-vue/src/components/menu/menu.ts

@@ -418,15 +418,14 @@ 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
         ? api.items.value[api.activeItemIndex.value].id === id
         : false
     })
 
-    let dataRef = ref<MenuItemDataRef['value']>({ disabled, textValue: '' })
+    let dataRef = ref<MenuItemDataRef['value']>({ 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,

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' })
     }
   },
 })

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

packages/@headlessui-vue/src/utils/render.ts

@@ -125,7 +125,7 @@ function _render({
   return h(as, passThroughProps, children)
 }
 
-function omit<T extends Record<any, any>>(object: T, keysToOmit: string[] = []) {
+export function omit<T extends Record<any, any>>(object: T, keysToOmit: string[] = []) {
   let clone = Object.assign({}, object)
   for (let key of keysToOmit) {
     if (key in clone) delete clone[key]