ECHOES-1008 Make keyboard navigation work for group label links

Author: david-cho-lerat-sonarsource

src/components/dropdown-menu/DropdownMenuGroupLabel.tsx

@@ -20,54 +20,85 @@
 
 import styled from '@emotion/styled';
 import * as radixDropdownMenu from '@radix-ui/react-dropdown-menu';
-import { forwardRef, ReactNode } from 'react';
+import { forwardRef } from 'react';
+import { LinkStandalone } from '../links';
+import { LinkBaseProps } from '../links/LinkTypes';
 
 import { cssVar } from '~utils/design-tokens';
 
 export interface DropdownMenuGroupLabelProps extends radixDropdownMenu.DropdownMenuLabelProps {
   /**
-   * Optional content to display at the end of the group label element.
+   * Optional link to display at the end of the group label element.
    */
-  suffix?: ReactNode;
+  link?: {
+    linkElement: LinkBaseProps['children'];
+    to: LinkBaseProps['to'];
+  };
 }
 
 export const DropdownMenuGroupLabel = forwardRef<HTMLDivElement, DropdownMenuGroupLabelProps>(
   (props, ref) => {
-    const { children, suffix, ...rest } = props;
+    const { children, link, ...rest } = props;
 
     return (
-      <StyledDropdownMenuGroupLabel ref={ref} {...rest}>
-        {/* Fragment wrapper to ensure single child structure, needed for compatibility */}
-        <>
+      <StyledWrapper>
+        <StyledDropdownMenuGroupLabel ref={ref} {...rest}>
           {children}
+        </StyledDropdownMenuGroupLabel>
 
-          {suffix && <StyledSuffix>{suffix}</StyledSuffix>}
-        </>
-      </StyledDropdownMenuGroupLabel>
+        {link && (
+          <StyledMenuItem asChild>
+            <StyledLink to={link.to}>{link.linkElement}</StyledLink>
+          </StyledMenuItem>
+        )}
+      </StyledWrapper>
     );
   },
 );
 
 DropdownMenuGroupLabel.displayName = 'DropdownMenuGroupLabel';
 
-const StyledDropdownMenuGroupLabel = styled(radixDropdownMenu.Label)`
+const StyledWrapper = styled.div`
   align-items: center;
-  color: ${cssVar('color-text-default')};
   display: flex;
-  font: ${cssVar('typography-text-small-semi-bold')};
   justify-content: space-between;
+`;
+
+const StyledDropdownMenuGroupLabel = styled(radixDropdownMenu.Label)`
+  color: ${cssVar('color-text-default')};
+  font: ${cssVar('typography-text-small-semi-bold')};
 
   padding: ${cssVar('dimension-space-100')} ${cssVar('dimension-space-150')}
     ${cssVar('dimension-space-50')};
 `;
 
 StyledDropdownMenuGroupLabel.displayName = 'StyledDropdownMenuGroupLabel';
 
-const StyledSuffix = styled.span`
-  align-items: center;
-  display: flex;
-  flex: 0 0 auto;
-  justify-content: flex-end;
+const StyledMenuItem = styled(radixDropdownMenu.Item)`
+  /* With asChild, these styles apply directly to the <a> tag */
+  background: none;
+  padding: ${cssVar('dimension-space-50')} ${cssVar('dimension-space-100')};
+
+  &:hover,
+  &[data-highlighted] {
+    background: none;
+  }
 `;
 
-StyledSuffix.displayName = 'StyledSuffix';
+const StyledLink = styled(LinkStandalone)`
+  /* Match the label typography (same as StyledDropdownMenuGroupLabel) */
+  font: ${cssVar('typography-text-small-semi-bold')};
+  margin-right: ${cssVar('dimension-space-100')};
+
+  /* Show focus ring only for keyboard navigation - && needed for extra specificity */
+  &&:focus-visible {
+    border-radius: ${cssVar('border-radius-200')};
+    outline: ${cssVar('color-focus-default')} solid ${cssVar('focus-border-width-default')};
+    outline-offset: ${cssVar('focus-border-offset-default')};
+  }
+
+  /* Hide focus ring on mouse hover - && needed for extra specificity */
+  &&:hover {
+    outline: none;
+  }
+`;

src/components/dropdown-menu/__tests__/DropdownMenu-test.tsx

@@ -25,7 +25,6 @@ import { ThemeProvider } from '~utils/theme';
 import { DropdownMenu, DropdownMenuAlign } from '..';
 import { Button } from '../../buttons';
 import { IconBell, IconCalendar } from '../../icons';
-import { LinkStandalone } from '../../links';
 
 const items = <DropdownMenu.ItemButton>An item</DropdownMenu.ItemButton>;
 const trigger = <Button>Trigger</Button>;
@@ -323,14 +322,13 @@ it('should render GroupLabel without suffix', () => {
   expect(screen.getByText('Item in group')).toBeVisible();
 });
 
-it('should render GroupLabel with suffix', () => {
+it('should render GroupLabel with link', () => {
   renderWithMemoryRouter(
     <DropdownMenu
       isOpen
       items={
         <>
-          <DropdownMenu.GroupLabel
-            suffix={<LinkStandalone to="/view-all">View all</LinkStandalone>}>
+          <DropdownMenu.GroupLabel link={{ linkElement: 'View all', to: '/view-all' }}>
             My Group
           </DropdownMenu.GroupLabel>
           <DropdownMenu.ItemButton>Item in group</DropdownMenu.ItemButton>
@@ -341,7 +339,11 @@ it('should render GroupLabel with suffix', () => {
   );
 
   expect(screen.getByText('My Group')).toBeVisible();
-  expect(screen.getByRole('link', { name: 'View all' })).toBeVisible();
+
+  const link = screen.getByRole('menuitem', { name: 'View all' });
+
+  expect(link).toBeVisible();
+  expect(link).toHaveAttribute('href', '/view-all');
 });
 
 it('should forward ref to GroupLabel', () => {

stories/dropdown-menu/DropdownMenu-stories.tsx

@@ -160,30 +160,28 @@ export const MenuWithADisabledButton: Story = {
   ),
 };
 
-export const MenuWithGroupLabelSuffix: Story = {
+export const MenuWithGroupLabelAction: Story = {
   render: () => (
     <BasicWrapper>
       <DropdownMenu
         items={
           <>
-            <DropdownMenu.GroupLabel
-              suffix={<LinkStandalone to="/enterprises">View all</LinkStandalone>}>
+            <DropdownMenu.GroupLabel link={{ linkElement: 'View all', to: '/enterprises' }}>
               Enterprises
             </DropdownMenu.GroupLabel>
 
             {enterpriseItems}
 
             <DropdownMenu.Separator />
 
-            <DropdownMenu.GroupLabel
-              suffix={<LinkStandalone to="/theme-settings">Configure</LinkStandalone>}>
+            <DropdownMenu.GroupLabel link={{ linkElement: 'Configure', to: '/theme-settings' }}>
               Theme
             </DropdownMenu.GroupLabel>
 
             {themeItems}
           </>
         }>
-        <MenuButton>Menu with suffix links</MenuButton>
+        <MenuButton>Menu with links</MenuButton>
       </DropdownMenu>
     </BasicWrapper>
   ),