feat: [M3-7589] - Improve Accessibility of Button Component (#10028)

Co-authored-by: Jaalah Ramos <jaalah.ramos@gmail.com>
linode · Jan 5, 2024 · 7f744f9 · 7f744f9
1 parent 3a1085b
commit 7f744f9
Show file tree

Hide file tree

Showing 33 changed files with 297 additions and 114 deletions.
diff --git a/packages/manager/.changeset/pr-10028-tech-stories-1704319234636.md b/packages/manager/.changeset/pr-10028-tech-stories-1704319234636.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Tech Stories
+---
+
+Improve Accessibility of Button Component ([#10028](https://github.com/linode/manager/pull/10028))
diff --git a/packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts b/packages/manager/cypress/e2e/core/linodes/resize-linode.spec.ts
@@ -33,7 +33,7 @@ describe('resize linode', () => {
       getClick('[id="g6-standard-4"]');
       cy.get('[data-qa-radio="warm"]').find('input').should('be.checked');
       cy.get('[data-testid="textfield-input"]').type(linode.label);
-      cy.get('[data-qa-resize="true"]').click();
+      cy.get('[data-qa-resize="true"]').should('be.enabled').click();
       cy.wait('@linodeResize');
 
       // TODO: Unified Migration: [M3-7115] - Replace with copy from API '../notifications.py'
@@ -60,7 +60,7 @@ describe('resize linode', () => {
       cy.get('[data-qa-radio="cold"]').click();
       cy.get('[data-qa-radio="cold"]').find('input').should('be.checked');
       cy.get('[data-testid="textfield-input"]').type(linode.label);
-      cy.get('[data-qa-resize="true"]').click();
+      cy.get('[data-qa-resize="true"]').should('be.enabled').click();
       cy.wait('@linodeResize');
 
       // TODO: Unified Migration: [M3-7115] - Replace with copy from API '../notifications.py'
@@ -109,7 +109,7 @@ describe('resize linode', () => {
         .should('be.checked')
         .should('be.disabled');
       cy.get('[data-testid="textfield-input"]').type(linode.label);
-      cy.get('[data-qa-resize="true"]').click();
+      cy.get('[data-qa-resize="true"]').should('be.enabled').click();
       cy.wait('@linodeResize');
 
       // TODO: Unified Migration: [M3-7115] - Replace with copy from API '../notifications.py'
@@ -135,7 +135,7 @@ describe('resize linode', () => {
       containsVisible('Linode 2 GB');
       getClick('[id="g6-standard-1"]');
       cy.get('[data-testid="textfield-input"]').type(linode.label);
-      cy.get('[data-qa-resize="true"]').click();
+      cy.get('[data-qa-resize="true"]').should('be.enabled').click();
       cy.wait('@linodeResize');
       // Failed to reduce the size of the linode
       cy.contains(
@@ -197,7 +197,7 @@ describe('resize linode', () => {
       containsVisible('Linode 2 GB');
       getClick('[id="g6-standard-1"]');
       cy.get('[data-testid="textfield-input"]').type(linode.label);
-      cy.get('[data-qa-resize="true"]').click();
+      cy.get('[data-qa-resize="true"]').should('be.enabled').click();
       cy.wait('@linodeResize');
       cy.contains(
         'Your Linode will soon be automatically powered off, migrated, and restored to its previous state (booted or powered off).'

diff --git a/packages/manager/cypress/support/e2e.ts b/packages/manager/cypress/support/e2e.ts
@@ -13,18 +13,51 @@
 // https://on.cypress.io/configuration
 // ***********************************************************
 
+import '@testing-library/cypress/add-commands';
 // Cypress command and assertion setup.
 import chaiString from 'chai-string';
-import '@testing-library/cypress/add-commands';
 import 'cypress-axe';
 import 'cypress-real-events/support';
-import './setup/login-command';
+
 import './setup/defer-command';
+import './setup/login-command';
 chai.use(chaiString);
 
+chai.use(function (chai, utils) {
+  utils.overwriteProperty(chai.Assertion.prototype, 'disabled', function () {
+    return function (this: Chai.AssertionStatic) {
+      const obj = utils.flag(this, 'object');
+      const isDisabled = Cypress.$(obj).is(':disabled');
+      const isAriaDisabled = Cypress.$(obj).attr('aria-disabled') === 'true';
+
+      this.assert(
+        isDisabled || isAriaDisabled,
+        'expected #{this} to be disabled',
+        'expected #{this} not to be disabled',
+        undefined
+      );
+    };
+  });
+
+  utils.overwriteProperty(chai.Assertion.prototype, 'enabled', function () {
+    return function (this: Chai.AssertionStatic) {
+      const obj = utils.flag(this, 'object');
+      const isDisabled = Cypress.$(obj).is(':disabled');
+      const isAriaDisabled = Cypress.$(obj).attr('aria-disabled') === 'true';
+
+      this.assert(
+        !isDisabled && !isAriaDisabled,
+        'expected #{this} to be enabled',
+        'expected #{this} not to be enabled',
+        undefined
+      );
+    };
+  });
+});
+
 // Test setup.
-import { trackApiRequests } from './setup/request-tracking';
 import { mockAccountRequest } from './setup/mock-account-request';
+import { trackApiRequests } from './setup/request-tracking';
 
 trackApiRequests();
 mockAccountRequest();
diff --git a/packages/manager/src/components/ActionMenu/ActionMenu.tsx b/packages/manager/src/components/ActionMenu/ActionMenu.tsx
@@ -9,6 +9,7 @@ import { convertToKebabCase } from 'src/utilities/convertToKebobCase';
 
 export interface Action {
   disabled?: boolean;
+  id?: string;
   onClick: () => void;
   title: string;
   tooltip?: string;

diff --git a/packages/manager/src/components/Button/Button.stories.tsx b/packages/manager/src/components/Button/Button.stories.tsx
@@ -29,6 +29,7 @@ const meta: Meta<typeof Button> = {
     compactY: false,
     disabled: false,
     loading: false,
+    onClick: action('onClick'),
     sx: {},
     tooltipAnalyticsEvent: action('tooltipAnalyticsEvent'),
     tooltipText: '',
@@ -91,3 +92,14 @@ export const LinkButton: Story = {
   // _args must be present in order to disable controls
   render: (_args) => <StyledLinkButton>Button</StyledLinkButton>,
 };
+
+/**
+ * Disabled Button w/ Tooltip
+ */
+export const DisabledTooltip: Story = {
+  args: {
+    disabled: true,
+    tooltipText: `You don't have permission to do this.`,
+  },
+  render: (args) => <Button {...args} />,
+};
diff --git a/packages/manager/src/components/Button/Button.test.tsx b/packages/manager/src/components/Button/Button.test.tsx
@@ -1,3 +1,4 @@
+import { fireEvent, screen } from '@testing-library/react';
 import React from 'react';
 
 import { renderWithTheme } from 'src/utilities/testHelpers';
@@ -19,10 +20,39 @@ describe('Button', () => {
 
   it('should render the HelpIcon when tooltipText is true', () => {
     const { getByTestId } = renderWithTheme(
-      <Button tooltipText="Test">Test</Button>
+      <Button disabled tooltipText="Test">
+        Test
+      </Button>
     );
 
     const helpIcon = getByTestId('HelpOutlineIcon');
     expect(helpIcon).toBeInTheDocument();
   });
+
+  it('should be disabled if loading', () => {
+    const { getByTestId } = renderWithTheme(<Button loading>Test</Button>);
+    const button = getByTestId('Button');
+    expect(button).toBeDisabled();
+  });
+
+  it('should have the aria-disabled attribute, instead of disabled attribute', () => {
+    const { getByTestId } = renderWithTheme(<Button disabled>Test</Button>);
+    const button = getByTestId('Button');
+    expect(button).not.toHaveAttribute('disabled');
+    expect(button).toHaveAttribute('aria-disabled', 'true');
+  });
+
+  it('should display the tooltip if disabled and tooltipText is true', async () => {
+    const { getByTestId } = renderWithTheme(
+      <Button disabled tooltipText="Test">
+        Test
+      </Button>
+    );
+
+    const button = getByTestId('Button');
+    expect(button).toHaveAttribute('aria-describedby', 'button-tooltip');
+
+    fireEvent.mouseOver(button);
+    await expect(screen.getByText('Test')).toBeInTheDocument();
+  });
 });
diff --git a/packages/manager/src/components/Button/Button.tsx b/packages/manager/src/components/Button/Button.tsx
@@ -1,10 +1,11 @@
+import HelpOutline from '@mui/icons-material/HelpOutline';
 import _Button, { ButtonProps as _ButtonProps } from '@mui/material/Button';
-import { Theme, styled, useTheme } from '@mui/material/styles';
+import { Theme, styled } from '@mui/material/styles';
 import { SxProps } from '@mui/system';
 import * as React from 'react';
 
 import Reload from 'src/assets/icons/reload.svg';
-import { TooltipIcon } from 'src/components/TooltipIcon';
+import { Tooltip } from 'src/components/Tooltip';
 
 import { rotate360 } from '../../styles/keyframes';
 import { omittedProps } from '../../utilities/omittedProps';
@@ -36,6 +37,8 @@ export interface ButtonProps extends _ButtonProps {
   loading?: boolean;
   /** The `sx` prop can be either object or function */
   sx?: SxProps<Theme>;
+  /** Pass specific CSS styling for the SVG icon. */
+  sxEndIcon?: SxProps<Theme>;
   /** Tooltip analytics event */
   tooltipAnalyticsEvent?: () => void;
   /** Tooltip text */
@@ -100,15 +103,15 @@ export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
       disabled,
       loading,
       sx,
+      sxEndIcon,
       tooltipAnalyticsEvent,
       tooltipText,
       ...rest
     },
     ref
   ) => {
-    const theme = useTheme();
     const color = buttonType === 'primary' ? 'primary' : 'secondary';
-    const sxTooltipIcon = { marginLeft: `-${theme.spacing()}` };
+    const showTooltip = disabled && Boolean(tooltipText);
 
     const variant =
       buttonType === 'primary' || buttonType === 'secondary'
@@ -117,34 +120,52 @@ export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
         ? 'outlined'
         : 'text';
 
-    return (
-      <React.Fragment>
-        <StyledButton
-          {...rest}
-          buttonType={buttonType}
-          className={className}
-          color={color}
-          compactX={compactX}
-          compactY={compactY}
-          disabled={disabled || loading}
-          loading={loading}
-          ref={ref}
-          sx={sx}
-          variant={variant}
-        >
-          <Span data-testid="loadingIcon">
-            {loading ? <Reload /> : children}
-          </Span>
-        </StyledButton>
-        {tooltipText && (
-          <TooltipIcon
-            status="help"
-            sxTooltipIcon={sxTooltipIcon}
-            text={tooltipText}
-            tooltipAnalyticsEvent={tooltipAnalyticsEvent}
-          />
-        )}
-      </React.Fragment>
+    const handleTooltipAnalytics = () => {
+      if (tooltipAnalyticsEvent) {
+        tooltipAnalyticsEvent();
+      }
+    };
+
+    const renderButton = (
+      <StyledButton
+        {...rest}
+        aria-describedby={
+          showTooltip ? 'button-tooltip' : rest['aria-describedby']
+        }
+        endIcon={
+          (showTooltip && <HelpOutline sx={sxEndIcon} />) || rest.endIcon
+        }
+        aria-disabled={disabled}
+        buttonType={buttonType}
+        className={className}
+        color={color}
+        compactX={compactX}
+        compactY={compactY}
+        data-testid={rest['data-testid'] || 'Button'}
+        disableRipple={disabled}
+        disabled={loading}
+        loading={loading}
+        onClick={disabled ? (e) => e.preventDefault() : rest.onClick}
+        onKeyDown={disabled ? (e) => e.preventDefault() : rest.onKeyDown}
+        ref={ref}
+        sx={sx}
+        variant={variant}
+      >
+        <Span data-testid="loadingIcon">{loading ? <Reload /> : children}</Span>
+      </StyledButton>
+    );
+
+    return showTooltip ? (
+      <Tooltip
+        data-testid="Tooltip"
+        id="button-tooltip"
+        onClick={handleTooltipAnalytics}
+        title={tooltipText}
+      >
+        {renderButton}
+      </Tooltip>
+    ) : (
+      renderButton
     );
   }
 );
diff --git a/packages/manager/src/components/Button/StyledActionButton.ts b/packages/manager/src/components/Button/StyledActionButton.ts
@@ -13,10 +13,12 @@ import { Button } from './Button';
 export const StyledActionButton = styled(Button, {
   label: 'StyledActionButton',
 })(({ theme, ...props }) => ({
-  '&:hover': {
-    backgroundColor: theme.palette.primary.main,
-    color: theme.name === 'dark' ? theme.color.black : theme.color.white,
-  },
+  ...(!props.disabled && {
+    '&:hover': {
+      backgroundColor: theme.palette.primary.main,
+      color: theme.name === 'dark' ? theme.color.black : theme.color.white,
+    },
+  }),
   fontFamily: latoWeb.normal,
   fontSize: '14px',
   lineHeight: '16px',

diff --git a/packages/manager/src/components/EnhancedNumberInput/EnhancedNumberInput.test.tsx b/packages/manager/src/components/EnhancedNumberInput/EnhancedNumberInput.test.tsx
@@ -68,7 +68,10 @@ describe('EnhancedNumberInput', () => {
 
     const input = getByTestId('textfield-input') as HTMLInputElement;
     expect(input.value).toBe('1');
-    expect(getByTestId('decrement-button')).toHaveAttribute('disabled');
+    expect(getByTestId('decrement-button')).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
   });
 
   it('should respect max values', () => {
@@ -78,15 +81,24 @@ describe('EnhancedNumberInput', () => {
 
     const input = getByTestId('textfield-input') as HTMLInputElement;
     expect(input.value).toBe('5');
-    expect(getByTestId('increment-button')).toHaveAttribute('disabled');
+    expect(getByTestId('increment-button')).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
   });
 
   it('should display buttons and input as disabled when given the corresponding prop', () => {
     const { getByTestId } = render(
       wrapWithTheme(<EnhancedNumberInput {...disabledProps} />)
     );
     expect(getByTestId('textfield-input')).toHaveAttribute('disabled');
-    expect(getByTestId('decrement-button')).toHaveAttribute('disabled');
-    expect(getByTestId('increment-button')).toHaveAttribute('disabled');
+    expect(getByTestId('decrement-button')).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
+    expect(getByTestId('increment-button')).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
   });
 });
diff --git a/packages/manager/src/components/InlineMenuAction/InlineMenuAction.tsx b/packages/manager/src/components/InlineMenuAction/InlineMenuAction.tsx
@@ -34,6 +34,7 @@ export const InlineMenuAction = (props: InlineMenuActionProps) => {
     onClick,
     tooltip,
     tooltipAnalyticsEvent,
+    ...rest
   } = props;
 
   if (href) {
@@ -47,12 +48,14 @@ export const InlineMenuAction = (props: InlineMenuActionProps) => {
   return (
     <StyledActionButton
       // TODO: We need to define what buttonType this will be in the future for now 'secondary' works...
+      aria-label={rest['aria-label']}
       buttonType="secondary"
       disabled={disabled}
       loading={loading}
       onClick={onClick}
       tooltipAnalyticsEvent={tooltipAnalyticsEvent}
       tooltipText={tooltip}
+      {...rest}
     >
       {actionText}
     </StyledActionButton>

diff --git a/packages/manager/src/components/LandingHeader/LandingHeader.test.tsx b/packages/manager/src/components/LandingHeader/LandingHeader.test.tsx
@@ -81,7 +81,10 @@ describe('LandingHeader', () => {
       />
     );
 
-    expect(getByText('Create My Entity').closest('button')).toBeDisabled();
+    expect(getByText('Create My Entity').closest('button')).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
   });
 
   it('should render custom crumb path based removeCrumbX prop', () => {