[Button] Make disabled buttons more accessible

.changeset/thirty-rockets-double.md

@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': minor
+---
+
+Make disabled buttons more accessible

polaris-react/src/components/BulkActions/tests/BulkActions.test.tsx

@@ -131,7 +131,7 @@ describe('<BulkActions />', () => {
       );
 
       expect(bulkActionsElement).toContainReactComponentTimes('button', 2, {
-        disabled: true,
+        'aria-disabled': true,
       });
     });
   });

polaris-react/src/components/Button/Button.scss

@@ -224,7 +224,7 @@
   }
 
   &:hover,
-  &:focus {
+  &:focus:not(.disabled) {
     @include recolor-icon(var(--p-interactive-hovered));
     color: var(--p-interactive-hovered);
     background: transparent;

polaris-react/src/components/Button/Button.tsx

@@ -17,6 +17,7 @@ import {Spinner} from '../Spinner';
 import {Popover} from '../Popover';
 import {ActionList} from '../ActionList';
 import {UnstyledButton, UnstyledButtonProps} from '../UnstyledButton';
+import {useDisableClick} from '../../utilities/use-disable-interaction';
 
 import styles from './Button.scss';
 
@@ -200,6 +201,8 @@ export function Button({
     setDisclosureActive((disclosureActive) => !disclosureActive);
   }, []);
 
+  const handleClick = useDisableClick(disabled, toggleDisclosureActive);
+
   let connectedDisclosureMarkup;
 
   if (connectedDisclosure) {
@@ -227,12 +230,13 @@ export function Button({
       <button
         type="button"
         className={connectedDisclosureClassName}
-        disabled={disabled}
+        aria-disabled={disabled}
         aria-label={disclosureLabel}
         aria-describedby={ariaDescribedBy}
         aria-checked={ariaChecked}
-        onClick={toggleDisclosureActive}
+        onClick={handleClick}
         onMouseUp={handleMouseUpByBlurring}
+        tabIndex={disabled ? -1 : undefined}
       >
         <span className={styles.Icon}>
           <Icon source={CaretDownMinor} />

polaris-react/src/components/Button/tests/Button.test.tsx

@@ -86,6 +86,21 @@ describe('<Button />', () => {
 
       expect(button).toContainReactComponent('a', {href: undefined});
     });
+
+    it('prevents default for onClick event when disabled', () => {
+      const onClick = jest.fn();
+      const button = mountWithApp(<Button disabled onClick={onClick} />);
+
+      const mockEvent = {
+        preventDefault: jest.fn(),
+        stopPropagation: jest.fn(),
+      };
+
+      button.find('button')!.trigger('onClick', mockEvent);
+
+      expect(mockEvent.preventDefault).toHaveBeenCalledTimes(1);
+      expect(mockEvent.stopPropagation).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('loading', () => {
@@ -259,7 +274,9 @@ describe('<Button />', () => {
       const button = mountWithApp(<Button connectedDisclosure={disclosure} />);
       const disclosureButton = button.findAll('button')[1];
 
-      expect(disclosureButton).toHaveReactProps({disabled: true});
+      expect(disclosureButton).toHaveReactProps({
+        'aria-disabled': true,
+      });
     });
 
     it('renders an ActionList with the actions set', () => {
@@ -280,6 +297,42 @@ describe('<Button />', () => {
         items: expect.arrayContaining(actions),
       });
     });
+
+    it('sets tabIndex to -1 on the disclosure button when disabled is true', () => {
+      const disclosure = {
+        disabled: true,
+        actions: [
+          {
+            content: 'Save and mark as ordered',
+          },
+        ],
+      };
+
+      const button = mountWithApp(<Button connectedDisclosure={disclosure} />);
+      const disclosureButton = button.findAll('button')[1];
+
+      expect(disclosureButton).toHaveReactProps({
+        tabIndex: -1,
+      });
+    });
+
+    it('sets tabIndex to undefined on the disclosure button when disabled is false', () => {
+      const disclosure = {
+        disabled: false,
+        actions: [
+          {
+            content: 'Save and mark as ordered',
+          },
+        ],
+      };
+
+      const button = mountWithApp(<Button connectedDisclosure={disclosure} />);
+      const disclosureButton = button.findAll('button')[1];
+
+      expect(disclosureButton).toHaveReactProps({
+        tabIndex: undefined,
+      });
+    });
   });
 
   describe('onClick()', () => {

polaris-react/src/components/UnstyledButton/UnstyledButton.tsx

@@ -2,6 +2,7 @@ import React from 'react';
 
 import type {BaseButton} from '../../types';
 import {handleMouseUpByBlurring} from '../../utilities/focus';
+import {useDisableClick} from '../../utilities/use-disable-interaction';
 import {UnstyledLink} from '../UnstyledLink';
 
 export interface UnstyledButtonProps extends BaseButton {
@@ -57,6 +58,8 @@ export function UnstyledButton({
     onTouchStart,
   };
 
+  const handleClick = useDisableClick(disabled, onClick);
+
   if (url) {
     buttonMarkup = disabled ? (
       // Render an `<a>` so toggling disabled/enabled state changes only the
@@ -77,8 +80,8 @@ export function UnstyledButton({
     buttonMarkup = (
       <button
         {...interactiveProps}
+        aria-disabled={disabled}
         type={submit ? 'submit' : 'button'}
-        disabled={disabled}
         aria-busy={loading ? true : undefined}
         aria-controls={ariaControls}
         aria-expanded={ariaExpanded}
@@ -88,6 +91,8 @@ export function UnstyledButton({
         onKeyDown={onKeyDown}
         onKeyUp={onKeyUp}
         onKeyPress={onKeyPress}
+        onClick={handleClick}
+        tabIndex={disabled ? -1 : undefined}
         {...rest}
       >
         {children}

polaris-react/src/components/UnstyledButton/tests/UnstyledButton.test.tsx

@@ -212,15 +212,15 @@ describe('<Button />', () => {
     it('passes to `button`', () => {
       const button = mountWithApp(<UnstyledButton disabled />);
       expect(button).toContainReactComponent('button', {
-        disabled: true,
+        'aria-disabled': true,
       });
     });
 
     it('does not pass to link', () => {
       const button = mountWithApp(<UnstyledButton url={mockUrl} disabled />);
       expect(button).toContainReactComponent('a');
       expect(button).not.toContainReactComponent('button', {
-        disabled: true,
+        'aria-disabled': true,
       });
     });
   });
@@ -363,6 +363,19 @@ describe('<Button />', () => {
       unstyledButton.find(UnstyledLink)!.trigger('onClick');
       expect(onClickSpy).toHaveBeenCalledTimes(1);
     });
+
+    it('prevents default when disabled is true', () => {
+      const onClickSpy = jest.fn();
+      const unstyledButton = mountWithApp(
+        <UnstyledButton onClick={onClickSpy} disabled />,
+      );
+      const mockEvent = {
+        preventDefault: jest.fn(),
+        stopPropagation: jest.fn(),
+      };
+      unstyledButton.find('button')!.trigger('onClick', mockEvent);
+      expect(mockEvent.preventDefault).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('onMouseEnter()', () => {
@@ -478,13 +491,32 @@ describe('<Button />', () => {
     });
   });
 
+  describe('tabIndex', () => {
+    it('sets tabIndex to -1 when disabled is true', () => {
+      const unstyledButton = mountWithApp(
+        <UnstyledButton disabled>Test</UnstyledButton>,
+      );
+      expect(unstyledButton.find('button')!.prop('tabIndex')).toBe(-1);
+    });
+
+    it('sets tabIndex to undefined by default', () => {
+      const unstyledButton = mountWithApp(
+        <UnstyledButton>Test</UnstyledButton>,
+      );
+      expect(unstyledButton.find('button')!.prop('tabIndex')).toBeUndefined();
+    });
+  });
+
   describe('onKeyDown()', () => {
     it('is called when a keydown event is registered on the button', () => {
       const spy = jest.fn();
       const unstyledButton = mountWithApp(
         <UnstyledButton onKeyDown={spy}>Test</UnstyledButton>,
       );
-      unstyledButton.find('button')!.trigger('onKeyDown');
+      const mockEvent = {
+        key: 'Enter',
+      };
+      unstyledButton.find('button')!.trigger('onKeyDown', mockEvent);
       expect(spy).toHaveBeenCalled();
     });
   });