ActionMenu: a11y fixes (batch 1) (#2099)

* Don't add aria-expanded=false * remove "Select a" from aria-label * remove tabIndex from AnchoredOverlay * update story to use interactive element * update snapshots * Create anchored-overlay-clever-mails-roll.md * update outdated label in test * Add describedby to milestone story
primer · Jun 29, 2022 · 40da598 · 40da598
1 parent f17446e
commit 40da598
Show file tree

Hide file tree

Showing 12 changed files with 29 additions and 40 deletions.
diff --git a/.changeset/anchored-overlay-clever-mails-roll.md b/.changeset/anchored-overlay-clever-mails-roll.md
@@ -0,0 +1,8 @@
+---
+"@primer/react": patch
+---
+
+AnchoredOverlay accessibility fixes
+- `aria-expanded` attribute is removed from anchor when overlay is not open
+- `tabIndex=0` is removed from anchor because it should only be used with interactive elements
+
diff --git a/src/AnchoredOverlay/AnchoredOverlay.tsx b/src/AnchoredOverlay/AnchoredOverlay.tsx
@@ -165,8 +165,7 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
           ref: anchorRef,
           id: anchorId,
           'aria-haspopup': 'true',
-          'aria-expanded': open,
-          tabIndex: 0,
+          'aria-expanded': open ? 'true' : undefined,
           onClick: onAnchorClick,
           onKeyDown: onAnchorKeyDown
         })}

diff --git a/src/__tests__/ActionMenu.test.tsx b/src/__tests__/ActionMenu.test.tsx
@@ -108,7 +108,7 @@ describe('ActionMenu', () => {
         <SingleSelection />
       </ThemeProvider>
     )
-    const button = component.getByLabelText('Select field type')
+    const button = component.getByLabelText('Field type')
     fireEvent.click(button)
 
     // select first item by role, that would close the menu
@@ -127,7 +127,7 @@ describe('ActionMenu', () => {
         <MixedSelection />
       </ThemeProvider>
     )
-    const button = component.getByLabelText('Select field type to group by')
+    const button = component.getByLabelText('Group by')
     fireEvent.click(button)
 
     expect(component.getByLabelText('Status')).toHaveAttribute('role', 'menuitemradio')

diff --git a/src/__tests__/__snapshots__/ActionMenu.test.tsx.snap b/src/__tests__/__snapshots__/ActionMenu.test.tsx.snap
@@ -125,13 +125,11 @@ exports[`ActionMenu renders consistently 1`] = `
   fontFamily="normal"
 >
   <button
-    aria-expanded={false}
     aria-haspopup="true"
     className="c1"
     id="react-aria-1"
     onClick={[Function]}
     onKeyDown={[Function]}
-    tabIndex={0}
     type="button"
   >
     <span

diff --git a/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap b/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap
@@ -80,13 +80,11 @@ exports[`AnchoredOverlay renders consistently 1`] = `
   fontFamily="normal"
 >
   <button
-    aria-expanded={false}
     aria-haspopup="true"
     className="c1"
     id="react-aria-1"
     onClick={[Function]}
     onKeyDown={[Function]}
-    tabIndex={0}
   >
     Anchor Button
   </button>
@@ -203,7 +201,6 @@ exports[`AnchoredOverlay should render consistently when open 1`] = `
       aria-haspopup="true"
       class="c1"
       id="react-aria-1"
-      tabindex="0"
     >
       Anchor Button
     </button>

diff --git a/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap b/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap
@@ -84,13 +84,11 @@ exports[`SelectPanel renders consistently 1`] = `
   fontFamily="normal"
 >
   <button
-    aria-expanded={false}
     aria-haspopup="true"
     className="c1"
     id="react-aria-1"
     onClick={[Function]}
     onKeyDown={[Function]}
-    tabIndex={0}
     type="button"
   >
     Select Items

diff --git a/src/__tests__/deprecated/__snapshots__/ActionMenu.test.tsx.snap b/src/__tests__/deprecated/__snapshots__/ActionMenu.test.tsx.snap
@@ -68,13 +68,11 @@ exports[`ActionMenu renders consistently 1`] = `
 }
 
 <button
-  aria-expanded={false}
   aria-haspopup="true"
   aria-label="menu"
   className="c0"
   id="react-aria-1"
   onClick={[Function]}
   onKeyDown={[Function]}
-  tabIndex={0}
 />
 `;
diff --git a/src/__tests__/deprecated/__snapshots__/DropdownMenu.test.tsx.snap b/src/__tests__/deprecated/__snapshots__/DropdownMenu.test.tsx.snap
@@ -72,13 +72,11 @@ exports[`DropdownMenu renders consistently 1`] = `
 }
 
 <button
-  aria-expanded={false}
   aria-haspopup="true"
   className="c0"
   id="react-aria-1"
   onClick={[Function]}
   onKeyDown={[Function]}
-  tabIndex={0}
   type="button"
 >
   <svg

diff --git a/src/stories/ActionList/examples.stories.tsx b/src/stories/ActionList/examples.stories.tsx
@@ -156,7 +156,7 @@ export function MultipleSelection(): JSX.Element {
 
       <p>This pattern appears in issues and pull requests to pick multiple assignees</p>
 
-      <ActionList selectionVariant="multiple" showDividers role="listbox" aria-label="Select assignees">
+      <ActionList selectionVariant="multiple" showDividers role="listbox" aria-label="Assignees">
         {users.map(user => (
           <ActionList.Item
             role="option"
@@ -192,7 +192,7 @@ export function Groups(): JSX.Element {
 
       <p>Grouped content with labels and description. This patterns appears in pull requests to pick a reviewer.</p>
 
-      <ActionList selectionVariant="multiple" role="menu" showDividers aria-label="Select reviewers">
+      <ActionList selectionVariant="multiple" role="menu" showDividers aria-label="Reviewers">
         <ActionList.Group title="Suggestions">
           {users.slice(0, 2).map(user => (
             <ActionList.Item
@@ -328,12 +328,7 @@ export function AsyncListWithSpinner(): JSX.Element {
       {results.length === 0 ? (
         <Text sx={{display: 'block', fontSize: 1, m: 2}}>No branches match that query</Text>
       ) : null}
-      <ActionList
-        selectionVariant="single"
-        role="listbox"
-        aria-label="Select a branch"
-        sx={{height: 208, overflow: 'auto'}}
-      >
+      <ActionList selectionVariant="single" role="listbox" aria-label="Branch" sx={{height: 208, overflow: 'auto'}}>
         {loading ? (
           <Box sx={{display: 'flex', justifyContent: 'center', pt: 2}}>
             <Spinner />

diff --git a/src/stories/ActionList/fixtures.stories.tsx b/src/stories/ActionList/fixtures.stories.tsx
@@ -208,7 +208,7 @@ export function DisabledStory(): JSX.Element {
     <>
       <h1>Disabled Items</h1>
       <ErsatzOverlay>
-        <ActionList selectionVariant="single" showDividers role="menu" aria-label="Select a project">
+        <ActionList selectionVariant="single" showDividers role="menu" aria-label="Project">
           {projects.map((project, index) => (
             <ActionList.Item
               key={index}
@@ -836,7 +836,7 @@ export function ChildWithSideEffects(): JSX.Element {
     <>
       <h1>Child with side effects</h1>
       <ErsatzOverlay>
-        <ActionList selectionVariant="multiple" role="listbox" aria-label="Select assignees">
+        <ActionList selectionVariant="multiple" role="listbox" aria-label="Assignees">
           <ActionList.Item selected={selected} onSelect={() => setSelected(!selected)} role="option">
             <ActionList.LeadingVisual>
               <Avatar src={`https://avatars.githubusercontent.com/${user.login}`} />

diff --git a/src/stories/ActionMenu/examples.stories.tsx b/src/stories/ActionMenu/examples.stories.tsx
@@ -89,7 +89,7 @@ export function SingleSelection(): JSX.Element {
       <p>This pattern has a single section with the selected value shown in the button</p>
 
       <ActionMenu>
-        <ActionMenu.Button aria-label="Select field type" leadingIcon={selectedType.icon}>
+        <ActionMenu.Button aria-label="Field type" leadingIcon={selectedType.icon}>
           {selectedType.name}
         </ActionMenu.Button>
         <ActionMenu.Overlay width="medium">
@@ -122,7 +122,7 @@ export function SingleSelectionWithPlaceholder(): JSX.Element {
       <p>This pattern has a placeholder in menu button when no value is selected yet</p>
 
       <ActionMenu>
-        <ActionMenu.Button aria-label="Select field type" leadingIcon={selectedType.icon}>
+        <ActionMenu.Button aria-label="Field type" leadingIcon={selectedType.icon}>
           {selectedType.name || 'Pick a field type'}
         </ActionMenu.Button>
         <ActionMenu.Overlay width="medium">
@@ -155,7 +155,8 @@ export function GroupsAndDescription(): JSX.Element {
       <Box sx={{width: 200}}>
         <ActionMenu>
           <ActionMenu.Button
-            aria-label="Select a milestone"
+            aria-label="Milestone"
+            aria-describedby="selected-milestone"
             variant="invisible"
             trailingIcon={GearIcon}
             sx={{
@@ -209,11 +210,11 @@ export function GroupsAndDescription(): JSX.Element {
           </ActionMenu.Overlay>
         </ActionMenu>
         {selectedMilestone ? (
-          <Text as="div" color="fg.muted" sx={{fontSize: 1, mt: 2}}>
+          <Text as="div" id="selected-milestone" color="fg.muted" sx={{fontSize: 1, mt: 2}}>
             {selectedMilestone.name}
           </Text>
         ) : (
-          <Text as="div" color="fg.muted" sx={{fontSize: 1, mt: 2}}>
+          <Text as="div" id="selected-milestone" color="fg.muted" sx={{fontSize: 1, mt: 2}}>
             No milestone
           </Text>
         )}
@@ -251,7 +252,7 @@ export function MultipleSelection(): JSX.Element {
       <Box sx={{width: 200}}>
         <ActionMenu>
           <ActionMenu.Button
-            aria-label="Select assignees"
+            aria-label="Assignees"
             variant="invisible"
             trailingIcon={GearIcon}
             sx={{
@@ -312,10 +313,7 @@ export function MixedSelection(): JSX.Element {
       </p>
 
       <ActionMenu>
-        <ActionMenu.Button
-          aria-label="Select field type to group by"
-          leadingIcon={selectedOption && selectedOption.icon}
-        >
+        <ActionMenu.Button aria-label="Group by" leadingIcon={selectedOption && selectedOption.icon}>
           {selectedOption ? `Group by ${selectedOption.text}` : 'Group items by'}
         </ActionMenu.Button>
         <ActionMenu.Overlay width="medium">

diff --git a/src/stories/ActionMenu/fixtures.stories.tsx b/src/stories/ActionMenu/fixtures.stories.tsx
@@ -207,9 +207,9 @@ export function CustomAnchor(): JSX.Element {
       <h2>Last option activated: {actionFired}</h2>
       <ActionMenu>
         <ActionMenu.Anchor>
-          <summary style={{cursor: 'pointer'}} aria-label="Open column options">
+          <button aria-label="Open column options">
             <KebabHorizontalIcon />
-          </summary>
+          </button>
         </ActionMenu.Anchor>
         <ActionMenu.Overlay>
           <ActionList>
@@ -505,7 +505,7 @@ export function MemexIteration(): JSX.Element {
             color: 'fg.muted',
             ':hover, :focus': {background: 'none !important', color: 'accent.fg'}
           }}
-          aria-label="Select iteration duration"
+          aria-label="Iteration duration"
         >
           {duration} {duration > 1 ? 'weeks' : 'week'}
         </ActionMenu.Button>
@@ -548,7 +548,7 @@ export function MemexAddColumn(): JSX.Element {
         </FormControl>
         <ActionMenu>
           <ActionMenu.Button
-            aria-label="Select field type"
+            aria-label="Field type"
             leadingIcon={selectedType.icon}
             sx={{
               gridTemplateColumns: 'min-content 1fr min-content',
@@ -582,7 +582,7 @@ export function MemexAddColumn(): JSX.Element {
           <ActionMenu>
             <ActionMenu.Button
               id="duration"
-              aria-label="Select field type"
+              aria-label="Field type"
               sx={{
                 textAlign: 'left',
                 ml: 2,