From 1475e7e780716bce2f016be3bb329a3b144c2936 Mon Sep 17 00:00:00 2001
From: Marliana Lara <mlara@redhat.com>
Date: Wed, 8 May 2024 15:00:06 -0400
Subject: [PATCH] Enable kebab dropdown keyboard accessibility

---
 framework/PageActions/PageActionDropdown.tsx | 245 +++++++++++--------
 1 file changed, 142 insertions(+), 103 deletions(-)

diff --git a/framework/PageActions/PageActionDropdown.tsx b/framework/PageActions/PageActionDropdown.tsx
index d86eadbaa5..895a794368 100644
--- a/framework/PageActions/PageActionDropdown.tsx
+++ b/framework/PageActions/PageActionDropdown.tsx
@@ -1,16 +1,18 @@
-import { ButtonVariant, Tooltip } from '@patternfly/react-core';
 import {
+  ButtonVariant,
+  Divider,
   Dropdown,
   DropdownItem,
-  DropdownPosition,
-  DropdownSeparator,
-  DropdownToggle,
-  KebabToggle,
-} from '@patternfly/react-core/deprecated';
-import { CircleIcon } from '@patternfly/react-icons';
+  DropdownList,
+  DropdownPopperProps,
+  Icon,
+  MenuToggle,
+  MenuToggleElement,
+  Tooltip,
+} from '@patternfly/react-core';
+import { CircleIcon, EllipsisVIcon } from '@patternfly/react-icons';
 import { ComponentClass, FunctionComponent, useEffect, useMemo, useState } from 'react';
 import { useTranslation } from 'react-i18next';
-import { Link } from 'react-router-dom';
 import styled from 'styled-components';
 import { PFColorE, getPatternflyColor } from '../components/pfcolors';
 import { getID } from '../hooks/useID';
@@ -40,7 +42,7 @@ interface PageActionDropdownProps<T extends object> {
   isDisabled?: string | undefined;
   label?: string;
   onOpen?: (label: string, open: boolean) => void;
-  position?: DropdownPosition;
+  position?: DropdownPopperProps['position'];
   selectedItem?: T;
   selectedItems?: T[];
   tooltip?: string;
@@ -97,64 +99,16 @@ export function PageActionDropdown<T extends object>(props: PageActionDropdownPr
   const id = getID(props.label ?? 'actions-dropdown');
 
   if (actions.length === 0) return <></>;
-  const Icon = icon;
-  const toggleIcon = Icon ? <Icon /> : label;
+
   const isPrimary =
     variant === ButtonVariant.primary || (hasBulkActions && !!selectedItems?.length);
   /** Turn primary button to secondary if there are items selected */
   const isSecondary =
     variant === ButtonVariant.primary && !hasBulkActions && !!selectedItems?.length;
-  const Toggle =
-    label || Icon ? (
-      <DropdownToggle
-        id="toggle-dropdown"
-        isDisabled={!!isDisabled}
-        onToggle={() => setDropdownOpen(!dropdownOpen)}
-        toggleVariant={isSecondary ? 'secondary' : isPrimary ? 'primary' : undefined}
-        toggleIndicator={Icon && iconOnly ? null : undefined}
-        style={isPrimary && !label ? { color: 'var(--pf-v5-global--Color--light-100)' } : {}}
-        icon={Icon ? <Icon /> : undefined}
-        data-cy={id}
-      >
-        {iconOnly ? undefined : label}
-      </DropdownToggle>
-    ) : (
-      <KebabToggle
-        className="toggle-kebab"
-        isDisabled={!!isDisabled}
-        onToggle={() => setDropdownOpen(!dropdownOpen)}
-        toggleVariant={isPrimary ? 'primary' : undefined}
-        style={isPrimary && !label ? { color: 'var(--pf-v5-global--Color--light-100)' } : {}}
-        data-cy={id}
-      >
-        {toggleIcon}
-      </KebabToggle>
-    );
-  const dropdown = (
-    <Dropdown
-      onSelect={() => setDropdownOpen(false)}
-      toggle={Toggle}
-      isOpen={dropdownOpen}
-      isPlain={!label || iconOnly}
-      dropdownItems={actions.map((action, index) => (
-        <PageDropdownActionItem
-          key={'label' in action ? action.label : `action-${index}`}
-          action={action}
-          selectedItems={selectedItems ?? []}
-          selectedItem={selectedItem}
-          hasIcons={hasIcons}
-          hasSwitches={hasSwitches}
-          index={index}
-          data-cy={id}
-        />
-      ))}
-      position={position}
-      // ZIndex 400 is needed for PF table stick headers
-      style={{ zIndex: dropdownOpen ? 400 : undefined }}
-    />
-  );
-  let tooltipContent;
+  const isKebab = Boolean(!label && !icon);
+  const CustomIcon = icon;
 
+  let tooltipContent;
   if (isDisabled) {
     tooltipContent = isDisabled;
   } else if (tooltip) {
@@ -165,9 +119,63 @@ export function PageActionDropdown<T extends object>(props: PageActionDropdownPr
     tooltipContent = undefined;
   }
 
+  const dropdownMenuLabel: string | JSX.Element | undefined =
+    iconOnly && CustomIcon ? (
+      <Icon>
+        <CustomIcon />
+      </Icon>
+    ) : (
+      label
+    );
+
   return (
     <Tooltip content={tooltipContent} trigger={tooltipContent ? undefined : 'manual'}>
-      {dropdown}
+      <Dropdown
+        isOpen={dropdownOpen}
+        onSelect={() => setDropdownOpen(false)}
+        onOpenChange={(isOpen) => setDropdownOpen(isOpen)}
+        popperProps={{
+          direction: 'down',
+          position: position ?? 'right',
+          enableFlip: true,
+        }}
+        toggle={(toggleRef: React.Ref<MenuToggleElement>) => (
+          <MenuToggle
+            ref={toggleRef}
+            id={isKebab ? 'toggle-kebab' : 'toggle-dropdown'}
+            isDisabled={!!isDisabled}
+            aria-label={isKebab ? 'kebab dropdown toggle' : 'dropdown toggle'}
+            variant={isSecondary ? 'secondary' : isPrimary ? 'primary' : 'plain'}
+            onClick={() => setDropdownOpen(!dropdownOpen)}
+            isExpanded={dropdownOpen}
+            style={isPrimary && !label ? { color: 'var(--pf-v5-global--Color--light-100)' } : {}}
+            icon={
+              CustomIcon ? (
+                <Icon>
+                  <CustomIcon />
+                </Icon>
+              ) : undefined
+            }
+          >
+            {dropdownMenuLabel ?? <EllipsisVIcon />}
+          </MenuToggle>
+        )}
+      >
+        <DropdownList>
+          {actions.map((action, index) => (
+            <PageDropdownActionItem
+              key={'label' in action ? action.label : `action-${index}`}
+              action={action}
+              selectedItems={selectedItems ?? []}
+              selectedItem={selectedItem}
+              hasIcons={hasIcons}
+              hasSwitches={hasSwitches}
+              index={index}
+              data-cy={id}
+            />
+          ))}
+        </DropdownList>
+      </Dropdown>
     </Tooltip>
   );
 }
@@ -187,8 +195,8 @@ function PageDropdownActionItem<T extends object>(props: {
 
   switch (action.type) {
     case PageActionType.Button: {
-      let Icon: ComponentClass | FunctionComponent | undefined = action.icon;
-      if (!Icon && hasIcons) Icon = TransparentIcon;
+      let CustomIcon: ComponentClass | FunctionComponent | undefined = action.icon;
+      if (!CustomIcon && hasIcons) CustomIcon = TransparentIcon;
       let tooltip;
 
       if (isDisabled) {
@@ -207,37 +215,54 @@ function PageDropdownActionItem<T extends object>(props: {
         isButtonDisabled = true;
       }
       return (
-        <Tooltip key={action.label} content={tooltip} trigger={tooltip ? undefined : 'manual'}>
-          <StyledDropdownItem $hasSwitches={hasSwitches} $isDanger={Boolean(action.isDanger)}>
-            <DropdownItem
-              icon={Icon ? <Icon /> : undefined}
-              onClick={() => {
-                switch (action.selection) {
-                  case PageActionSelection.None:
-                    action.onClick();
-                    break;
-                  case PageActionSelection.Single:
-                    if (selectedItem) action.onClick(selectedItem);
-                    break;
-                  case PageActionSelection.Multiple:
-                    if (selectedItems) action.onClick(selectedItems);
-                    break;
-                }
-              }}
-              isAriaDisabled={isButtonDisabled}
-              id={getID(action)}
-              data-cy={getID(action)?.split('.').join('-')}
-            >
-              {action.label}
-            </DropdownItem>
-          </StyledDropdownItem>
-        </Tooltip>
+        <StyledDropdownItem $hasSwitches={hasSwitches} $isDanger={Boolean(action.isDanger)}>
+          <DropdownItem
+            id={getID(action)}
+            data-cy={getID(action)?.split('.').join('-')}
+            isDisabled={Boolean(isButtonDisabled)}
+            isAriaDisabled={Boolean(isButtonDisabled)}
+            onClick={() => {
+              switch (action.selection) {
+                case PageActionSelection.None:
+                  action.onClick();
+                  break;
+                case PageActionSelection.Single:
+                  if (selectedItem) action.onClick(selectedItem);
+                  break;
+                case PageActionSelection.Multiple:
+                  if (selectedItems) action.onClick(selectedItems);
+                  break;
+              }
+            }}
+            style={{
+              color:
+                action.isDanger && !isDisabled ? getPatternflyColor(PFColorE.Danger) : undefined,
+            }}
+            tooltipProps={{
+              content: tooltip,
+              trigger: tooltip ? undefined : 'manual',
+            }}
+          >
+            {CustomIcon ? (
+              <Icon
+                size="lg"
+                iconSize="md"
+                style={{
+                  paddingRight: '.5rem',
+                }}
+              >
+                <CustomIcon />
+              </Icon>
+            ) : undefined}
+            {action.label}
+          </DropdownItem>
+        </StyledDropdownItem>
       );
     }
 
     case PageActionType.Link: {
-      let Icon: ComponentClass | FunctionComponent | undefined = action.icon;
-      if (!Icon && hasIcons) Icon = TransparentIcon;
+      let CustomIcon: ComponentClass | FunctionComponent | undefined = action.icon;
+      if (!CustomIcon && hasIcons) CustomIcon = TransparentIcon;
       const tooltip = isDisabled ? isDisabled : action.tooltip;
       let to: string;
 
@@ -256,17 +281,31 @@ function PageDropdownActionItem<T extends object>(props: {
       }
 
       return (
-        <Tooltip key={action.label} content={tooltip} trigger={tooltip ? undefined : 'manual'}>
-          <DropdownItem
-            isAriaDisabled={Boolean(isDisabled)}
-            icon={Icon ? <Icon /> : undefined}
-            component={<Link to={to}>{action.label}</Link>}
-            style={{
-              color:
-                action.isDanger && !isDisabled ? getPatternflyColor(PFColorE.Danger) : undefined,
-            }}
-          />
-        </Tooltip>
+        <DropdownItem
+          isDisabled={Boolean(isDisabled)}
+          isAriaDisabled={Boolean(isDisabled)}
+          to={to}
+          style={{
+            color: action.isDanger && !isDisabled ? getPatternflyColor(PFColorE.Danger) : undefined,
+          }}
+          tooltipProps={{
+            content: tooltip,
+            trigger: tooltip ? undefined : 'manual',
+          }}
+        >
+          {CustomIcon ? (
+            <Icon
+              size="lg"
+              iconSize="md"
+              style={{
+                paddingRight: '.5rem',
+              }}
+            >
+              <CustomIcon />
+            </Icon>
+          ) : undefined}
+          {action.label}
+        </DropdownItem>
       );
     }
 
@@ -295,7 +334,7 @@ function PageDropdownActionItem<T extends object>(props: {
     }
 
     case PageActionType.Seperator:
-      return <DropdownSeparator key={`separator-${index}`} />;
+      return <Divider component="li" key={`separator-${index}`} />;
   }
 }