From 202261c84f063a09ac1d4bf029038158fe992016 Mon Sep 17 00:00:00 2001
From: Cee Chen <constance.chen@elastic.co>
Date: Thu, 16 Nov 2023 20:07:42 -0800
Subject: [PATCH] Fix tooltip aria-describedby ID not correctly applying to
 buttons due to multiple children

---
 changelogs/upcoming/7373.md                   |  4 ++
 .../default_item_action.test.tsx.snap         | 34 +++++------
 .../basic_table/default_item_action.test.tsx  |  4 +-
 .../basic_table/default_item_action.tsx       | 57 +++++++++++--------
 4 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/changelogs/upcoming/7373.md b/changelogs/upcoming/7373.md
index bc25d49a32fc..f317a785f999 100644
--- a/changelogs/upcoming/7373.md
+++ b/changelogs/upcoming/7373.md
@@ -9,3 +9,7 @@
 
 - Deprecated `EuiContextMenuItem`'s `toolTipTitle` prop. Use `toolTipProps.title` instead
 - Deprecated `EuiContextMenuItem`'s `toolTipPosition` prop. Use `toolTipProps.position` instead
+
+**Accessibility**
+
+- Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not correctly reading out action descriptions to screen readers
diff --git a/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap b/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap
index 5afeeba49e4a..4feeceba8273 100644
--- a/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap
+++ b/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap
@@ -21,22 +21,24 @@ exports[`DefaultItemAction renders an EuiButtonEmpty when \`type="button" 1`] =
 </span>
 `;
 
-exports[`DefaultItemAction renders an EuiButtonIcon when \`type="icon"\` 1`] = `
-<span
-  class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
->
-  <button
-    aria-labelledby="generated-id"
-    class="euiButtonIcon emotion-euiButtonIcon-xs-empty-primary"
-    type="button"
+exports[`DefaultItemAction renders an EuiButtonIcon with screen reader text when \`type="icon"\` 1`] = `
+<div>
+  <span
+    class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
   >
-    <span
-      aria-hidden="true"
-      class="euiButtonIcon__icon"
-      color="inherit"
-      data-euiicon-type="trash"
-    />
-  </button>
+    <button
+      aria-labelledby="generated-id"
+      class="euiButtonIcon emotion-euiButtonIcon-xs-empty-primary"
+      type="button"
+    >
+      <span
+        aria-hidden="true"
+        class="euiButtonIcon__icon"
+        color="inherit"
+        data-euiicon-type="trash"
+      />
+    </button>
+  </span>
   <span
     class="emotion-euiScreenReaderOnly"
     id="generated-id"
@@ -45,5 +47,5 @@ exports[`DefaultItemAction renders an EuiButtonIcon when \`type="icon"\` 1`] = `
       action1
     </span>
   </span>
-</span>
+</div>
 `;
diff --git a/src/components/basic_table/default_item_action.test.tsx b/src/components/basic_table/default_item_action.test.tsx
index a95ef449e80c..bafd8918835f 100644
--- a/src/components/basic_table/default_item_action.test.tsx
+++ b/src/components/basic_table/default_item_action.test.tsx
@@ -43,7 +43,7 @@ describe('DefaultItemAction', () => {
     expect(container.firstChild).toMatchSnapshot();
   });
 
-  it('renders an EuiButtonIcon when `type="icon"`', () => {
+  it('renders an EuiButtonIcon with screen reader text when `type="icon"`', () => {
     const action: IconButtonAction<Item> = {
       name: <span>action1</span>,
       description: 'action 1',
@@ -59,7 +59,7 @@ describe('DefaultItemAction', () => {
 
     const { container } = render(<DefaultItemAction {...props} />);
 
-    expect(container.firstChild).toMatchSnapshot();
+    expect(container).toMatchSnapshot();
   });
 
   it('renders an EuiButtonEmpty if no type is specified', () => {
diff --git a/src/components/basic_table/default_item_action.tsx b/src/components/basic_table/default_item_action.tsx
index 398319dedb1a..d1732b9fb1c8 100644
--- a/src/components/basic_table/default_item_action.tsx
+++ b/src/components/basic_table/default_item_action.tsx
@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import React, { ReactElement } from 'react';
+import React, { ReactElement, ReactNode } from 'react';
 
 import { isString } from '../../services/predicate';
 import {
@@ -63,29 +63,32 @@ export const DefaultItemAction = <T,>({
   const dataTestSubj = callWithItemIfFunction(item)(action['data-test-subj']);
 
   const ariaLabelId = useGeneratedHtmlId();
+  let ariaLabelledBy: ReactNode;
+
   if (action.type === 'icon') {
     if (!icon) {
       throw new Error(`Cannot render item action [${action.name}]. It is configured to render as an icon but no
       icon is provided. Make sure to set the 'icon' property of the action`);
     }
     button = (
-      <>
-        <EuiButtonIcon
-          className={className}
-          aria-labelledby={ariaLabelId}
-          isDisabled={!enabled}
-          color={color}
-          iconType={icon}
-          onClick={onClick}
-          href={href}
-          target={action.target}
-          data-test-subj={dataTestSubj}
-        />
-        {/* actionContent (action.name) is a ReactNode and must be rendered to an element and referenced by ID for screen readers */}
-        <EuiScreenReaderOnly>
-          <span id={ariaLabelId}>{actionContent}</span>
-        </EuiScreenReaderOnly>
-      </>
+      <EuiButtonIcon
+        className={className}
+        aria-labelledby={ariaLabelId}
+        isDisabled={!enabled}
+        color={color}
+        iconType={icon}
+        onClick={onClick}
+        href={href}
+        target={action.target}
+        data-test-subj={dataTestSubj}
+      />
+    );
+    // actionContent (action.name) is a ReactNode and must be rendered
+    // to an element and referenced by ID for screen readers
+    ariaLabelledBy = (
+      <EuiScreenReaderOnly>
+        <span id={ariaLabelId}>{actionContent}</span>
+      </EuiScreenReaderOnly>
     );
   } else {
     button = (
@@ -106,11 +109,19 @@ export const DefaultItemAction = <T,>({
     );
   }
 
-  return enabled && tooltipContent ? (
-    <EuiToolTip content={tooltipContent} delay="long">
-      {button}
-    </EuiToolTip>
+  return enabled ? (
+    <>
+      <EuiToolTip content={tooltipContent} delay="long">
+        {button}
+      </EuiToolTip>
+      {/* SR text has to be rendered outside the tooltip,
+      otherwise EuiToolTip's own aria-labelledby won't properly clone */}
+      {ariaLabelledBy}
+    </>
   ) : (
-    button
+    <>
+      {button}
+      {ariaLabelledBy}
+    </>
   );
 };