Improve aria accessibility (#592)

* encode expected `aria-expanded` behaviour * ensure `aria-expanded` has the correct value `aria-expanded` can be in 3 different states: | Value | Description | | ------------------- | -------------------------------------------------------------------------- | | false | The grouping element this element owns or controls is collapsed. | | true | The grouping element this element owns or controls is expanded. | | undefined (default) | The element does not own or control a grouping element that is expandable. | Ref: https://www.w3.org/TR/wai-aria-1.2/#aria-expanded Fixes: #580 * ensure `disabled` prop in Vue is not rendered when `false` * update changelog
tailwindlabs · Jun 4, 2021 · 045b843 · 045b843
1 parent 11c461e
commit 045b843
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improve `disabled` and `tabindex` prop handling ([#512](https://github.com/tailwindlabs/headlessui/pull/512))
 - Improve React peer dependency version range ([#544](https://github.com/tailwindlabs/headlessui/pull/544))
 - Improve types for the `open` prop in the `Dialog` component ([#550](https://github.com/tailwindlabs/headlessui/pull/550))
+- Improve `aria-expanded` logic ([#592](https://github.com/tailwindlabs/headlessui/pull/592))
 
 ## [Unreleased - Vue]
 
@@ -29,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - 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))
+- Improve `aria-expanded` logic ([#592](https://github.com/tailwindlabs/headlessui/pull/592))
 
 ## [@headlessui/react@v1.2.0] - 2021-05-10
 

diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx
@@ -220,7 +220,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
     ref: buttonRef,
     id: state.buttonId,
     type: 'button',
-    'aria-expanded': state.disclosureState === DisclosureStates.Open ? true : undefined,
+    'aria-expanded': props.disabled ? undefined : state.disclosureState === DisclosureStates.Open,
     'aria-controls': state.linkedPanel ? state.panelId : undefined,
     onKeyDown: handleKeyDown,
     onKeyUp: handleKeyUp,

diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx
@@ -356,7 +356,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
     type: 'button',
     'aria-haspopup': true,
     'aria-controls': state.optionsRef.current?.id,
-    'aria-expanded': state.listboxState === ListboxStates.Open ? true : undefined,
+    'aria-expanded': state.disabled ? undefined : state.listboxState === ListboxStates.Open,
     'aria-labelledby': labelledby,
     disabled: state.disabled,
     onKeyDown: handleKeyDown,

diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx
@@ -297,7 +297,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
     type: 'button',
     'aria-haspopup': true,
     'aria-controls': state.itemsRef.current?.id,
-    'aria-expanded': state.menuState === MenuStates.Open ? true : undefined,
+    'aria-expanded': props.disabled ? undefined : state.menuState === MenuStates.Open,
     onKeyDown: handleKeyDown,
     onKeyUp: handleKeyUp,
     onClick: handleClick,

diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx
@@ -440,7 +440,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
         ref: buttonRef,
         id: state.buttonId,
         type: 'button',
-        'aria-expanded': state.popoverState === PopoverStates.Open ? true : undefined,
+        'aria-expanded': props.disabled ? undefined : state.popoverState === PopoverStates.Open,
         'aria-controls': state.panel ? state.panelId : undefined,
         onKeyDown: handleKeyDown,
         onKeyUp: handleKeyUp,

diff --git a/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-react/src/test-utils/accessibility-assertions.ts
@@ -62,12 +62,20 @@ export function assertMenuButton(
 
       case MenuState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case MenuState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:
@@ -326,12 +334,20 @@ export function assertListboxButton(
 
       case ListboxState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case ListboxState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:
@@ -628,12 +644,20 @@ export function assertDisclosureButton(
 
       case DisclosureState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case DisclosureState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:
@@ -752,12 +776,20 @@ export function assertPopoverButton(
 
       case PopoverState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case PopoverState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:

diff --git a/packages/@headlessui-vue/src/components/disclosure/disclosure.ts b/packages/@headlessui-vue/src/components/disclosure/disclosure.ts
@@ -94,8 +94,11 @@ export let DisclosureButton = defineComponent({
     let propsWeControl = {
       id: this.id,
       type: 'button',
-      'aria-expanded': api.disclosureState.value === DisclosureStates.Open ? true : undefined,
+      'aria-expanded': this.$props.disabled
+        ? undefined
+        : api.disclosureState.value === DisclosureStates.Open,
       'aria-controls': this.ariaControls,
+      disabled: this.$props.disabled ? true : undefined,
       onClick: this.handleClick,
       onKeydown: this.handleKeyDown,
       onKeyup: this.handleKeyUp,

diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts
@@ -267,11 +267,13 @@ export let ListboxButton = defineComponent({
       type: 'button',
       'aria-haspopup': true,
       'aria-controls': dom(api.optionsRef)?.id,
-      'aria-expanded': api.listboxState.value === ListboxStates.Open ? true : undefined,
+      'aria-expanded': api.disabled.value
+        ? undefined
+        : api.listboxState.value === ListboxStates.Open,
       'aria-labelledby': api.labelRef.value
         ? [dom(api.labelRef)?.id, this.id].join(' ')
         : undefined,
-      disabled: api.disabled.value,
+      disabled: api.disabled.value === true ? true : undefined,
       onKeydown: this.handleKeyDown,
       onKeyup: this.handleKeyUp,
       onClick: this.handleClick,

diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts
@@ -187,7 +187,7 @@ export let MenuButton = defineComponent({
       type: 'button',
       'aria-haspopup': true,
       'aria-controls': dom(api.itemsRef)?.id,
-      'aria-expanded': api.menuState.value === MenuStates.Open ? true : undefined,
+      'aria-expanded': this.$props.disabled ? undefined : api.menuState.value === MenuStates.Open,
       onKeydown: this.handleKeyDown,
       onKeyup: this.handleKeyUp,
       onClick: this.handleClick,

diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts
@@ -203,8 +203,11 @@ export let PopoverButton = defineComponent({
           ref: 'el',
           id: api.buttonId,
           type: 'button',
-          'aria-expanded': api.popoverState.value === PopoverStates.Open ? true : undefined,
+          'aria-expanded': this.$props.disabled
+            ? undefined
+            : api.popoverState.value === PopoverStates.Open,
           'aria-controls': dom(api.panel) ? api.panelId : undefined,
+          disabled: this.$props.disabled ? true : undefined,
           onKeydown: this.handleKeyDown,
           onKeyup: this.handleKeyUp,
           onClick: this.handleClick,

diff --git a/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts b/packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts
@@ -62,12 +62,20 @@ export function assertMenuButton(
 
       case MenuState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case MenuState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:
@@ -326,12 +334,20 @@ export function assertListboxButton(
 
       case ListboxState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case ListboxState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:
@@ -628,12 +644,20 @@ export function assertDisclosureButton(
 
       case DisclosureState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case DisclosureState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default:
@@ -752,12 +776,20 @@ export function assertPopoverButton(
 
       case PopoverState.InvisibleHidden:
         expect(button).toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       case PopoverState.InvisibleUnmounted:
         expect(button).not.toHaveAttribute('aria-controls')
-        expect(button).not.toHaveAttribute('aria-expanded')
+        if (button.hasAttribute('disabled')) {
+          expect(button).not.toHaveAttribute('aria-expanded')
+        } else {
+          expect(button).toHaveAttribute('aria-expanded', 'false')
+        }
         break
 
       default: