[EuiKeyPadMenuItem] Moves beta badge tooltip props to a wrapping EuiT…

…oolTip (#5541) * Idea: Move betaBadge tooltip contents to a EuiToolTip wrapper that wraps the keypad menu item only if necessary * Added `betaBadgeTooltipProps` to allow extending the outer wrapper if necassary * Added `aria-hidden`
elastic · Jan 18, 2022 · 4960592 · 4960592
1 parent ca4435a
commit 4960592
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 76 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,19 +1,24 @@
 ## [`main`](https://github.com/elastic/eui/tree/main)
 
-- Added `lettering` glyph to `EuiIcon` ([#5525](https://github.com/elastic/eui/pull/5525))
-- Updated the outline color in `euiCustomControlFocused` mixin to use 
-`$euiFocusRingColor` instead of `currentColor` ([#5479](https://github.com/elastic/eui/pull/5479))
 - **[Beta]** Added `optimize` build as a lighter weight option more suited to prodcution environments ([#5527](https://github.com/elastic/eui/pull/5527))
+- Added `lettering` glyph to `EuiIcon` ([#5525](https://github.com/elastic/eui/pull/5525))
+- Updated the outline color in `euiCustomControlFocused` mixin to use `$euiFocusRingColor` instead of `currentColor` ([#5479](https://github.com/elastic/eui/pull/5479))
+- Added `betaBadgeTooltipProps` to `EuiKeyPadMenuItem` to extend the wrapping `EuiToolTip` ([#5541](https://github.com/elastic/eui/pull/5541))
 
 **Bug fixes**
 
 - Updated the outline color in `euiCustomControlFocused` mixin to use `$euiFocusRingColor` instead of `currentColor` ([#5479](https://github.com/elastic/eui/pull/5479))
 - Fixed keyboard navigation in `EuiDataGrid` not fully scrolling cells into view ([#5515](https://github.com/elastic/eui/pull/5515))
+- Fixed `EuiKeyPadMenuItem` accessibility issue where there was a nested focusable element ([#5541](https://github.com/elastic/eui/pull/5541))
 
 **Deprecations**
 
 - Deprecated `data-gridcell-id` from `EuiDataGrid` in favor of 4 new and more flexible props - `data-gridcell-column-id`, `data-gridcell-column-index`, `data-gridcell-row-index`, and `data-gridcell-visible-row-index` ([#5515](https://github.com/elastic/eui/pull/5515))
 
+**Breaking changes**
+
+- `EuiKeyPadMenuItem` now wraps itself with `EuiToolTip` when `betaBadgeLabel` is supplied forcing top element style props to be passed via `betaBadgeTooltipProps` ([#5541](https://github.com/elastic/eui/pull/5541))
+
 ## [`45.0.0`](https://github.com/elastic/eui/tree/v45.0.0)
 
 - Added virtulized rendering option to `EuiSelectableList` with `isVirtualized` ([#5521](https://github.com/elastic/eui/pull/5521))

diff --git a/src/components/badge/beta_badge/beta_badge.tsx b/src/components/badge/beta_badge/beta_badge.tsx
@@ -210,7 +210,7 @@ export const EuiBetaBadge: FunctionComponent<EuiBetaBadgeProps> = ({
           content={tooltipContent}
           title={title || label}
         >
-          <Fragment>{content}</Fragment>
+          {content}
         </EuiToolTip>
       );
     } else {
@@ -224,7 +224,7 @@ export const EuiBetaBadge: FunctionComponent<EuiBetaBadgeProps> = ({
           content={tooltipContent}
           title={title || label}
         >
-          <span tabIndex={0} className={classes} {...rest}>
+          <span tabIndex={0} className={classes} role="button" {...rest}>
             {icon || label}
           </span>
         </EuiToolTip>

diff --git a/src/components/key_pad_menu/__snapshots__/key_pad_menu_item.test.tsx.snap b/src/components/key_pad_menu/__snapshots__/key_pad_menu_item.test.tsx.snap
@@ -94,96 +94,140 @@ exports[`EuiKeyPadMenuItem is rendered 1`] = `
 `;
 
 exports[`EuiKeyPadMenuItem props betaBadge renders 1`] = `
-<button
-  class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
-  type="button"
+<span
+  class="euiToolTipAnchor"
 >
-  <span
-    class="euiKeyPadMenuItem__inner"
+  <button
+    class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
+    type="button"
   >
     <span
-      class="euiBetaBadge euiBetaBadge--singleLetter euiBetaBadge--subdued euiBetaBadge--small euiKeyPadMenuItem__betaBadge"
-      title="Beta"
-    >
-      B
-    </span>
-    <span
-      class="euiKeyPadMenuItem__icon"
+      class="euiKeyPadMenuItem__inner"
     >
-      Icon
+      <span
+        aria-hidden="true"
+        class="euiBetaBadge euiBetaBadge--singleLetter euiBetaBadge--subdued euiBetaBadge--small euiKeyPadMenuItem__betaBadge"
+        title="B"
+      >
+        B
+      </span>
+      <span
+        class="euiKeyPadMenuItem__icon"
+      >
+        Icon
+      </span>
+      <span
+        class="euiKeyPadMenuItem__label"
+      >
+        Label
+      </span>
     </span>
+  </button>
+</span>
+`;
+
+exports[`EuiKeyPadMenuItem props betaBadge renders extra betaBadgeTooltipProps 1`] = `
+<span
+  class="euiToolTipAnchor customTooltipClass"
+>
+  <button
+    class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
+    type="button"
+  >
     <span
-      class="euiKeyPadMenuItem__label"
+      class="euiKeyPadMenuItem__inner"
     >
-      Label
+      <span
+        aria-hidden="true"
+        class="euiBetaBadge euiBetaBadge--singleLetter euiBetaBadge--subdued euiBetaBadge--small euiKeyPadMenuItem__betaBadge"
+        title="B"
+      >
+        B
+      </span>
+      <span
+        class="euiKeyPadMenuItem__icon"
+      >
+        Icon
+      </span>
+      <span
+        class="euiKeyPadMenuItem__label"
+      >
+        Label
+      </span>
     </span>
-  </span>
-</button>
+  </button>
+</span>
 `;
 
 exports[`EuiKeyPadMenuItem props betaBadge renders with betaBadgeIconType 1`] = `
-<button
-  class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
-  type="button"
+<span
+  class="euiToolTipAnchor"
 >
-  <span
-    class="euiKeyPadMenuItem__inner"
+  <button
+    class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
+    type="button"
   >
     <span
-      class="euiBetaBadge euiBetaBadge--iconOnly euiBetaBadge--singleLetter euiBetaBadge--subdued euiBetaBadge--small euiKeyPadMenuItem__betaBadge"
-      title="Beta"
+      class="euiKeyPadMenuItem__inner"
     >
       <span
         aria-hidden="true"
-        class="euiBetaBadge__icon"
-        color="inherit"
-        data-euiicon-type="bolt"
-      />
-    </span>
-    <span
-      class="euiKeyPadMenuItem__icon"
-    >
-      Icon
-    </span>
-    <span
-      class="euiKeyPadMenuItem__label"
-    >
-      Label
+        class="euiBetaBadge euiBetaBadge--iconOnly euiBetaBadge--singleLetter euiBetaBadge--subdued euiBetaBadge--small euiKeyPadMenuItem__betaBadge"
+        title="B"
+      >
+        <span
+          aria-hidden="true"
+          class="euiBetaBadge__icon"
+          color="inherit"
+          data-euiicon-type="bolt"
+        />
+      </span>
+      <span
+        class="euiKeyPadMenuItem__icon"
+      >
+        Icon
+      </span>
+      <span
+        class="euiKeyPadMenuItem__label"
+      >
+        Label
+      </span>
     </span>
-  </span>
-</button>
+  </button>
+</span>
 `;
 
 exports[`EuiKeyPadMenuItem props betaBadge renders with betaBadgeTooltipContent 1`] = `
-<button
-  class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
-  type="button"
+<span
+  class="euiToolTipAnchor"
 >
-  <span
-    class="euiKeyPadMenuItem__inner"
+  <button
+    class="euiKeyPadMenuItem euiKeyPadMenuItem--hasBetaBadge"
+    type="button"
   >
     <span
-      class="euiToolTipAnchor"
+      class="euiKeyPadMenuItem__inner"
     >
       <span
+        aria-hidden="true"
         class="euiBetaBadge euiBetaBadge--singleLetter euiBetaBadge--subdued euiBetaBadge--small euiKeyPadMenuItem__betaBadge"
-        tabindex="0"
+        title="B"
       >
         B
       </span>
+      <span
+        class="euiKeyPadMenuItem__icon"
+      >
+        Icon
+      </span>
+      <span
+        class="euiKeyPadMenuItem__label"
+      >
+        Label
+      </span>
     </span>
-    <span
-      class="euiKeyPadMenuItem__icon"
-    >
-      Icon
-    </span>
-    <span
-      class="euiKeyPadMenuItem__label"
-    >
-      Label
-    </span>
-  </span>
-</button>
+  </button>
+</span>
 `;
 
 exports[`EuiKeyPadMenuItem props isDisabled renders with href 1`] = `

diff --git a/src/components/key_pad_menu/key_pad_menu_item.test.tsx b/src/components/key_pad_menu/key_pad_menu_item.test.tsx
@@ -110,6 +110,21 @@ describe('EuiKeyPadMenuItem', () => {
 
         expect(component).toMatchSnapshot();
       });
+
+      test('renders extra betaBadgeTooltipProps', () => {
+        const component = render(
+          <EuiKeyPadMenuItem
+            betaBadgeLabel="Beta"
+            betaBadgeTooltipContent="Content"
+            betaBadgeTooltipProps={{ anchorClassName: 'customTooltipClass' }}
+            label="Label"
+          >
+            Icon
+          </EuiKeyPadMenuItem>
+        );
+
+        expect(component).toMatchSnapshot();
+      });
     });
   });
 

diff --git a/src/components/key_pad_menu/key_pad_menu_item.tsx b/src/components/key_pad_menu/key_pad_menu_item.tsx
@@ -29,6 +29,7 @@ import { getSecureRelForTarget, useGeneratedHtmlId } from '../../services';
 import { IconType } from '../icon';
 import { EuiRadio, EuiCheckbox } from '../form';
 import { validateHref } from '../../services/security/href_validator';
+import { EuiToolTip, EuiToolTipProps } from '../tool_tip';
 
 export type EuiKeyPadMenuItemCheckableType = 'single' | 'multi';
 
@@ -70,6 +71,12 @@ type EuiKeyPadMenuItemPropsForUncheckable = {
    * Add a description to the beta badge (will appear in a tooltip)
    */
   betaBadgeTooltipContent?: ReactNode;
+  /**
+   * Extends the wrapping EuiToolTip props when `betaBadgeLabel` is provided
+   */
+  betaBadgeTooltipProps?: Partial<
+    Omit<EuiToolTipProps, 'title' | 'content' | 'delay'>
+  >;
   /**
    * Use `onClick` instead when the item is not `checkable`
    */
@@ -153,6 +160,7 @@ export const EuiKeyPadMenuItem: FunctionComponent<EuiKeyPadMenuItemProps> = ({
   betaBadgeLabel,
   betaBadgeTooltipContent,
   betaBadgeIconType,
+  betaBadgeTooltipProps,
   href,
   rel,
   target,
@@ -225,25 +233,18 @@ export const EuiKeyPadMenuItem: FunctionComponent<EuiKeyPadMenuItemProps> = ({
 
     return (
       <EuiBetaBadge
+        // Since we move the tooltip contents to a wrapping EuiToolTip,
+        // this badge is purely visual therefore we can safely hide it from screen readers
+        aria-hidden="true"
         size="s"
         color="subdued"
         className="euiKeyPadMenuItem__betaBadge"
         label={betaBadgeLabel.charAt(0)}
-        title={betaBadgeLabel}
         iconType={betaBadgeIconType}
-        tooltipContent={betaBadgeTooltipContent}
       />
     );
   };
 
-  const renderContent = () => (
-    <span className="euiKeyPadMenuItem__inner">
-      {checkable ? renderCheckableElement() : renderBetaBadge()}
-      <span className="euiKeyPadMenuItem__icon">{children}</span>
-      <span className="euiKeyPadMenuItem__label">{label}</span>
-    </span>
-  );
-
   const relObj: {
     disabled?: boolean;
     type?: string;
@@ -268,15 +269,32 @@ export const EuiKeyPadMenuItem: FunctionComponent<EuiKeyPadMenuItemProps> = ({
     relObj['aria-pressed'] = isSelected;
   }
 
-  return (
+  const button = (
     <Element
       className={classes}
       {...(relObj as ElementType)}
       {...(rest as ElementType)}
       // Unable to get past `LegacyRef` conflicts
       ref={buttonRef as Ref<any>}
     >
-      {renderContent()}
+      <span className="euiKeyPadMenuItem__inner">
+        {checkable ? renderCheckableElement() : renderBetaBadge()}
+        <span className="euiKeyPadMenuItem__icon">{children}</span>
+        <span className="euiKeyPadMenuItem__label">{label}</span>
+      </span>
     </Element>
   );
+
+  return betaBadgeLabel ? (
+    <EuiToolTip
+      {...betaBadgeTooltipProps}
+      title={betaBadgeLabel}
+      content={betaBadgeTooltipContent}
+      delay="long"
+    >
+      {button}
+    </EuiToolTip>
+  ) : (
+    button
+  );
 };