Button: Beefing up accessibility tests and cleaning up state manageme…

…nt (#17155)

* Button: Beefing up accessibility tests and cleaning up state management.

* Change files

* Updating change file.

* Updating snapshots.

* Removing unused imports.

* Fixing lint error.

Co-authored-by: KHMakoto <humberto_makoto@hotmail.com>

change/@fluentui-react-button-e1acb3ed-0311-42c0-b9ec-468f986666b9.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Button: Beefing up accessibility tests and cleaning up state management.",
+  "packageName": "@fluentui/react-button",
+  "email": "Humberto.Morimoto@microsoft.com",
+  "dependentChangeType": "patch"
+}

packages/a11y-testing/src/definitions/index.ts

@@ -2,7 +2,9 @@ export * from './Button/buttonBehaviorDefinition';
 export * from './Button/buttonGroupBehaviorDefinition';
 export * from './Button/toggleButtonBehaviorDefinition';
 export * from './Link/linkBehaviorDefinition';
-export * from './Popup/popupBehaviorDefinition';
 export * from './MenuButton/menuButtonBehaviorDefinition';
-export * from './Pill/pillBehaviorDefinition';
 export * from './Pill/pillActionBehaviorDefinition';
+export * from './Pill/pillBehaviorDefinition';
+export * from './Popup/popupBehaviorDefinition';
+
+export * from './react-button/buttonAccessibilityBehaviorDefinition';

packages/a11y-testing/src/definitions/react-button/buttonAccessibilityBehaviorDefinition.ts

@@ -0,0 +1,76 @@
+import { Rule } from './../../types';
+import { BehaviorRule } from './../../rules/rules';
+
+export const buttonAccessibilityBehaviorDefinition: Rule[] = [
+  BehaviorRule.root()
+    .doesNotHaveAttribute('role')
+    .doesNotHaveAttribute('tabindex')
+    .description(`if element is rendered as a default 'button'.`),
+  // BehaviorRule.root()
+  //   .forProps({ href: '#' })
+  //   .hasAttribute('role', 'button')
+  //   .hasAttribute('tabindex', '0')
+  //   .description(`if element has href and is rendered as an 'anchor'.`),
+  BehaviorRule.root()
+    .forProps({ as: 'div' })
+    .hasAttribute('data-is-focusable', 'true')
+    .hasAttribute('role', 'button')
+    .hasAttribute('tabindex', '0')
+    .description(`if element type is other than the defaults 'anchor' and 'button'.`),
+  BehaviorRule.root()
+    .forProps({ disabled: true })
+    .hasAttribute('disabled')
+    .description(`if element is rendered as a default 'button' and is disabled.`),
+  // BehaviorRule.root()
+  //   .forProps({ disabled: true, href: '#' })
+  //   .doesNotHaveAttribute('disabled')
+  //   .description(`if element has href and is rendered as an 'anchor' and is disabled.`),
+  BehaviorRule.root()
+    .forProps({ as: 'div', disabled: true })
+    .doesNotHaveAttribute('disabled')
+    .description(`if element type is other than the defaults 'anchor' and 'button' and is disabled.`),
+  // BehaviorRule.root()
+  //   .forProps({ disabledFocusable: true })
+  //   .doesNotHaveAttribute('disabled')
+  //   .hasAttribute('aria-disabled', 'true')
+  //   .description(`if element is rendered as a default 'button' and is disabled but focusable.`),
+  // BehaviorRule.root()
+  //   .forProps({ disabledFocusable: true, href: '#' })
+  //   .doesNotHaveAttribute('disabled')
+  //   .hasAttribute('aria-disabled', 'true')
+  //   .hasAttribute('tabindex', '0')
+  //   .description(`if element has href and is rendered as an 'anchor' and is disabled but focusable.`),
+  // BehaviorRule.root()
+  //   .forProps({ as: 'div', disabledFocusable: true })
+  //   .doesNotHaveAttribute('disabled')
+  //   .hasAttribute('aria-disabled', 'true')
+  //   .hasAttribute('tabindex', '0')
+  //   .description(`if element type is other than the defaults 'anchor' and 'button' and is disabled but focusable.`),
+  // BehaviorRule.root()
+  //   .forProps({ disabled: true, disabledFocusable: true })
+  //   .doesNotHaveAttribute('disabled')
+  //   .hasAttribute('aria-disabled', 'true')
+  //   .description(`if element is rendered as a default 'button' and is disabled but focusable.`),
+  // BehaviorRule.root()
+  //   .forProps({ disabled: true, disabledFocusable: true, href: '#' })
+  //   .doesNotHaveAttribute('disabled')
+  //   .hasAttribute('aria-disabled', 'true')
+  //   .hasAttribute('tabindex', '0')
+  //   .description(`if element has href and is rendered as an 'anchor' and is disabled but focusable.`),
+  // BehaviorRule.root()
+  //   .forProps({ as: 'div', disabled: true, disabledFocusable: true })
+  //   .doesNotHaveAttribute('disabled')
+  //   .hasAttribute('aria-disabled', 'true')
+  //   .hasAttribute('tabindex', '0')
+  //   .description(`if element type is other than the defaults 'anchor' and 'button' and is disabled but focusable.`),
+  BehaviorRule.root()
+    .forProps({ as: 'div' })
+    .pressSpaceKey()
+    .verifyOnclickExecution()
+    .description(`if element type is other than the defaults 'anchor' and 'button'.`),
+  BehaviorRule.root()
+    .forProps({ as: 'div' })
+    .pressEnterKey()
+    .verifyOnclickExecution()
+    .description(`if element type is other than the defaults 'anchor' and 'button'.`),
+];

packages/react-button/etc/react-button.api.md

@@ -140,7 +140,7 @@ export const renderCompoundButton: (state: CompoundButtonState) => JSX.Element;
 export const useButton: (props: ButtonProps, ref: React.Ref<HTMLElement>, defaultProps?: ButtonProps | undefined) => ButtonState;
 
 // @public
-export const useButtonState: (draftState: ButtonState) => void;
+export const useButtonState: (state: ButtonState) => ButtonState;
 
 // @public (undocumented)
 export const useButtonStyles: (state: ButtonState, selectors: ButtonStyleSelectors) => void;

packages/react-button/src/components/Button/Button.test.tsx

@@ -1,15 +1,13 @@
 import * as React from 'react';
-import { mount, shallow, ReactWrapper } from 'enzyme';
-import * as renderer from 'react-test-renderer';
+import { mount, ReactWrapper } from 'enzyme';
+import {
+  buttonAccessibilityBehaviorDefinition,
+  // buttonBehaviorDefinition,
+  validateBehavior,
+  ComponentTestFacade,
+} from '@fluentui/a11y-testing';
 import { isConformant } from '../../common/isConformant';
 import { Button } from './Button';
-// import { validateBehavior, ComponentTestFacade, buttonBehaviorDefinition } from '@fluentui/a11y-testing';
-
-describe('Button (isConformant)', () =>
-  isConformant({
-    Component: Button,
-    displayName: 'Button',
-  }));
 
 describe('Button', () => {
   let wrapper: ReactWrapper | undefined;
@@ -21,19 +19,48 @@ describe('Button', () => {
     }
   });
 
-  /**
-   * Note: see more visual regression tests for Button in /apps/vr-tests.
-   */
-  it('renders a default state', () => {
-    const component = renderer.create(<Button>Default button</Button>);
-    const tree = component.toJSON();
+  isConformant({
+    Component: Button,
+    displayName: 'Button',
+  });
+
+  describe('meets accessibility requirements', () => {
+    const testFacade = new ComponentTestFacade(Button, {});
+
+    // let errors;
+    const errors = validateBehavior(buttonAccessibilityBehaviorDefinition, testFacade);
+    expect(errors).toEqual([]);
+
+    // errors = validateBehavior(buttonBehaviorDefinition, testFacade);
+    // expect(errors).toEqual([]);
+  });
+
+  it('renders a default button', () => {
+    wrapper = mount(<Button>This is a button</Button>);
+    const button = wrapper.find('button');
+    const anchor = wrapper.find('a');
+    expect(button.length).toBe(1);
+    expect(anchor.length).toBe(0);
+
+    const tree = button.debug();
     expect(tree).toMatchSnapshot();
   });
 
+  // it('renders as an anchor when href is provided', () => {
+  //   wrapper = mount(<Button href="https://www.bing.com">This is a button</Button>);
+  //   const button = wrapper.find('button');
+  //   const anchor = wrapper.find('a');
+  //   expect(button.length).toBe(0);
+  //   expect(anchor.length).toBe(1);
+
+  //   const tree = anchor.debug();
+  //   expect(tree).toMatchSnapshot();
+  // });
+
   it('can be focused', () => {
     const rootRef = React.createRef<HTMLButtonElement>();
 
-    wrapper = mount(<Button ref={rootRef}>Focus me</Button>);
+    wrapper = mount(<Button ref={rootRef}>This is a button</Button>);
 
     expect(typeof rootRef.current).toEqual('object');
     expect(document.activeElement).not.toEqual(rootRef.current);
@@ -43,32 +70,56 @@ describe('Button', () => {
     expect(document.activeElement).toEqual(rootRef.current);
   });
 
+  // it('can be focused when rendered as an anchor', () => {
+  //   const rootRef = React.createRef<HTMLButtonElement>();
+
+  //   wrapper = mount(
+  //     <Button href="https://www.bing.com" ref={rootRef}>
+  //       This is a button
+  //     </Button>,
+  //   );
+
+  //   expect(typeof rootRef.current).toEqual('object');
+  //   expect(document.activeElement).not.toEqual(rootRef.current);
+
+  //   rootRef.current?.focus();
+
+  //   expect(document.activeElement).toEqual(rootRef.current);
+  // });
+
+  it('can trigger a function by being clicked', () => {
+    const onClick = jest.fn();
+    wrapper = mount(<Button onClick={onClick}>This is a button</Button>);
+
+    wrapper.find('button').simulate('click');
+
+    expect(onClick).toHaveBeenCalled();
+  });
+
   it('does not trigger a function by being clicked when button is disabled', () => {
     const onClick = jest.fn();
-    shallow(
+    wrapper = mount(
       <Button disabled onClick={onClick}>
-        I am a button
+        This is a button
       </Button>,
-    ).simulate('click');
+    );
+
+    wrapper.find('button').simulate('click');
 
     expect(onClick).not.toHaveBeenCalled();
   });
 
   // it(`does not trigger a function by being clicked when button is disabled, even when disabledFocusable has been
   //     provided`, () => {
   //   const onClick = jest.fn();
-  //   shallow(
+  //   wrapper = mount(
   //     <Button disabled disabledFocusable onClick={onClick}>
-  //       I am a button
+  //       This is a button
   //     </Button>,
-  //   ).simulate('click');
-  //
-  //   expect(onClick).not.toHaveBeenCalled();
-  // });
+  //   );
 
-  // describe('AccessibilityButtonBehavior', () => {
-  //   const testFacade = new ComponentTestFacade(Button, {});
-  //   const errors = validateBehavior(buttonBehaviorDefinition, testFacade);
-  //   expect(errors).toEqual([]);
+  //   wrapper.find('button').simulate('click');
+
+  //   expect(onClick).not.toHaveBeenCalled();
   // });
 });

packages/react-button/src/components/Button/__snapshots__/Button.test.tsx.snap

@@ -1,14 +1,10 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Button renders a default state 1`] = `
-<button
-  className=""
-  onKeyDown={[Function]}
->
-  <span
-    className=""
-  >
-    Default button
+exports[`Button renders a default button 1`] = `
+"<button onClick={[Function]} onKeyDown={[Function]} aria-disabled={[undefined]} disabled={[undefined]} className=\\"\\">
+  <Component />
+  <span className=\\"\\">
+    This is a button
   </span>
-</button>
+</button>"
 `;

packages/react-button/src/components/Button/useButtonState.ts

@@ -4,40 +4,56 @@ import { ButtonState } from './Button.types';
 
 /**
  * The useButton hook processes the Button draft state.
- * @param draftState - Button draft state to mutate.
+ * @param state - Button draft state to mutate.
  */
-export const useButtonState = (draftState: ButtonState) => {
-  if (draftState.as !== 'button') {
-    draftState.role = 'button';
+export const useButtonState = (state: ButtonState): ButtonState => {
+  const { as, disabled, /*disabledFocusable,*/ onClick, onKeyDown: onKeyDownCallback } = state;
 
-    if (draftState.as !== 'a') {
-      const { onClick: onClickCallback, onKeyDown: onKeyDownCallback } = draftState;
+  const onNonAnchorOrButtonKeyDown = (ev: React.KeyboardEvent<HTMLElement>) => {
+    onKeyDownCallback?.(ev);
 
-      draftState.tabIndex = 0;
+    const keyCode = getCode(ev);
+    if (!ev.defaultPrevented && onClick && (keyCode === EnterKey || keyCode === SpacebarKey)) {
+      // Translate the keydown enter/space to a click.
+      ev.preventDefault();
+      ev.stopPropagation();
 
-      draftState.onKeyDown = (ev: React.KeyboardEvent<HTMLElement>) => {
-        if (onKeyDownCallback) {
-          onKeyDownCallback(ev);
-        }
+      onClick((ev as unknown) as React.MouseEvent<HTMLAnchorElement | HTMLButtonElement | HTMLElement>);
+    }
+  };
 
-        const keyCode = getCode(ev);
-        if (!ev.defaultPrevented && onClickCallback && (keyCode === EnterKey || keyCode === SpacebarKey)) {
-          // Translate the keydown enter/space to a click.
-          ev.preventDefault();
-          ev.stopPropagation();
+  // Adjust props depending on the root type.
+  if (typeof as === 'string') {
+    // Add 'role=button' and 'tabIndex=0' for all non-button elements.
+    if (as !== 'button') {
+      state.role = 'button';
+      state.tabIndex = disabled /*&& !disabledFocusable*/ ? undefined : 0;
 
-          (ev.target as HTMLElement).click();
-        }
-      };
+      // Add keydown event handler for all other non-anchor elements.
+      if (as !== 'a') {
+        state.onKeyDown = onNonAnchorOrButtonKeyDown;
+      }
     }
   }
-
-  // Disallow click and keyboard events when component is disabled and eat events when disabledFocusable is set to true.
-  const { disabled, /* disabledFocusable, */ onKeyDown } = draftState;
-  if (disabled) {
-    draftState.onClick = undefined;
+  // Add keydown event handler, 'role=button' and 'tabIndex=0' for all other elements.
+  else {
+    state.onKeyDown = onNonAnchorOrButtonKeyDown;
+    state.role = 'button';
+    state.tabIndex = disabled /*&& !disabledFocusable*/ ? undefined : 0;
   }
-  draftState.onKeyDown = (ev: React.KeyboardEvent<HTMLElement>) => {
+
+  // Disallow click event when component is disabled and eat events when disabledFocusable is set to true.
+  state.onClick = (ev: React.MouseEvent<HTMLElement>) => {
+    if (disabled) {
+      ev.preventDefault();
+    } else {
+      onClick?.(ev);
+    }
+  };
+
+  // Disallow keydown event when component is disabled and eat events when disabledFocusable is set to true.
+  const { onKeyDown } = state;
+  state.onKeyDown = (ev: React.KeyboardEvent<HTMLElement>) => {
     const keyCode = getCode(ev);
     if (disabled && (keyCode === EnterKey || keyCode === SpacebarKey)) {
       ev.preventDefault();
@@ -47,6 +63,9 @@ export const useButtonState = (draftState: ButtonState) => {
     }
   };
 
-  draftState['aria-disabled'] = disabled /* || disabledFocusable*/;
-  draftState.disabled = draftState.as === 'button' ? disabled /* && !disabledFocusable */ : undefined;
+  // Set the aria-disabled and disabled props correctly.
+  state['aria-disabled'] = disabled /*|| disabledFocusable*/;
+  state.disabled = as === 'button' ? disabled /* && !disabledFocusable*/ : undefined;
+
+  return state;
 };