feat: add accessibility features to popover control (#792)

meganaconley · web-flow · commit 6ac4e11114d0 · 2019-11-19T13:25:21.000-08:00
* feat: add accessibility features to popover control

* disable keyboard press handler on components that use complex controls

* update role handling
diff --git a/src/ComboboxInput/ComboboxInput.js b/src/ComboboxInput/ComboboxInput.js
@@ -37,6 +37,7 @@ const ComboboxInput = React.forwardRef(({ placeholder, menu, compact, className,
                         </span>
                     </div>
                 }
+                disableKeyPressHandler
                 disableStyles={disableStyles}
                 noArrow />
         </div>
diff --git a/src/Dropdown/__snapshots__/Dropdown.test.js.snap b/src/Dropdown/__snapshots__/Dropdown.test.js.snap
@@ -16,8 +16,12 @@ exports[`<Dropdown /> create dropdown component 1`] = `
         className="fd-popover__control"
       >
         <button
+          aria-haspopup={true}
           className="fd-button fd-dropdown__control"
           onClick={[Function]}
+          onKeyPress={[Function]}
+          role="button"
+          tabIndex={0}
         >
           Select
         </button>
@@ -43,8 +47,12 @@ exports[`<Dropdown /> create dropdown component 2`] = `
         className="fd-popover__control"
       >
         <button
+          aria-haspopup={true}
           className="fd-button fd-button--compact fd-dropdown__control"
           onClick={[Function]}
+          onKeyPress={[Function]}
+          role="button"
+          tabIndex={0}
         >
           Select
         </button>
@@ -70,8 +78,12 @@ exports[`<Dropdown /> create dropdown component 3`] = `
         className="fd-popover__control"
       >
         <button
+          aria-haspopup={true}
           className="fd-button fd-dropdown__control"
           onClick={[Function]}
+          onKeyPress={[Function]}
+          role="button"
+          tabIndex={0}
         >
           Select
         </button>
@@ -98,8 +110,12 @@ exports[`<Dropdown /> create dropdown component 4`] = `
         className="fd-popover__control"
       >
         <button
+          aria-haspopup={true}
           className="fd-button sap-icon--filter fd-dropdown__control"
           onClick={[Function]}
+          onKeyPress={[Function]}
+          role="button"
+          tabIndex={0}
         >
           Select
         </button>
@@ -126,9 +142,13 @@ exports[`<Dropdown /> create dropdown component 5`] = `
         className="fd-popover__control"
       >
         <button
+          aria-haspopup={true}
           className="fd-button sap-icon--filter is-disabled fd-dropdown__control"
           disabled={true}
           onClick={[Function]}
+          onKeyPress={[Function]}
+          role="button"
+          tabIndex={0}
         >
           Select
         </button>
diff --git a/src/Identifier/Identifier.js b/src/Identifier/Identifier.js
@@ -5,7 +5,7 @@ import React from 'react';
 import withStyles from '../utils/WithStyles/WithStyles';
 import { IDENTIFIER_MODIFIERS, IDENTIFIER_SIZES } from '../utils/constants';
 
-const Identifier = React.forwardRef(({ glyph, size, modifier, color, label, backgroundImageUrl, children, className, disableStyles, ...props }, ref) => {
+const Identifier = React.forwardRef(({ glyph, size, modifier, color, label, backgroundImageUrl, children, className, disableStyles, role, ...props }, ref) => {
     const styles = {
         backgroundImage: `url(${backgroundImageUrl})`
     };
@@ -21,7 +21,12 @@ const Identifier = React.forwardRef(({ glyph, size, modifier, color, label, back
         className
     );
 
-    const ariaRole = !children ? 'presentation' : '';
+    let ariaRole;
+    if (role) {
+        ariaRole = role;
+    } else {
+        ariaRole = !children ? 'presentation' : '';
+    }
 
     return (
         <span
@@ -48,13 +53,15 @@ Identifier.propTypes = {
     glyph: PropTypes.string,
     label: PropTypes.string,
     modifier: PropTypes.oneOf(IDENTIFIER_MODIFIERS),
+    role: PropTypes.string,
     size: PropTypes.oneOf(IDENTIFIER_SIZES)
 };
 
 Identifier.propDescriptions = {
     backgroundImageUrl: 'Image URL.',
     color: 'Applies a background color.',
     label: 'Localized text for label.',
+    role: 'Applies an aria-role. Set to button if Identifier opens a Popover or Modal.',
     size: 'Size of the image. These sizes are available: **xxs** (extra extra small) - 20px, **xs** (extra small) - 28px, **s** (small) - 32px, **m** (medium) - 48px, **l** (large) - 64px, **xl** (extra lagre) - 88px, and **xxl** (extra extra large). Default matches the base font size (14px).'
 };
 
diff --git a/src/LocalizationEditor/LocalizationEditor.js b/src/LocalizationEditor/LocalizationEditor.js
@@ -110,6 +110,7 @@ const LocalizationEditor = React.forwardRef(({ control, menu, id, compact, texta
                         </span>
                     </div>
                 }
+                disableKeyPressHandler
                 disableStyles={disableStyles}
                 id={id}
                 noArrow />
diff --git a/src/Popover/Popover.js b/src/Popover/Popover.js
@@ -1,5 +1,6 @@
 import chain from 'chain-function';
 import classnames from 'classnames';
+import keycode from 'keycode';
 import Popper from '../utils/_Popper';
 import { POPPER_PLACEMENTS } from '../utils/constants';
 import PropTypes from 'prop-types';
@@ -15,6 +16,18 @@ class Popover extends Component {
         };
     }
 
+    isButton = (node) => {
+        if (!node) {
+            return false;
+        }
+        if (typeof node.type === 'string') {
+            return node.type.toLowerCase() === 'button';
+        } else if (node.type.displayName) {
+            return node.type.displayName.toLowerCase() === 'button';
+        }
+        return false;
+    };
+
     triggerBody = () => {
         if (!this.props.disabled) {
             this.setState(prevState => ({
@@ -31,9 +44,23 @@ class Popover extends Component {
         }
     };
 
+    handleKeyPress = (event, node, onClickFunctions) => {
+        if (!this.isButton(node)) {
+            switch (keycode(event)) {
+                case 'enter':
+                case 'space':
+                    event.preventDefault();
+                    onClickFunctions();
+                    break;
+                default:
+            }
+        }
+    }
+
     render() {
         const {
             disableEdgeDetection,
+            disableKeyPressHandler,
             disableStyles,
             onClickOutside,
             onEscapeKey,
@@ -52,9 +79,21 @@ class Popover extends Component {
             onClickFunctions = chain(this.triggerBody, control.props.onClick);
         }
 
-        const referenceComponent = React.cloneElement(control, {
+        let controlProps = {
             onClick: onClickFunctions
-        });
+        };
+
+        if (!disableKeyPressHandler) {
+            controlProps = {
+                ...controlProps,
+                tabIndex: 0,
+                role: 'button',
+                'aria-haspopup': true,
+                onKeyPress: (event) => this.handleKeyPress(event, control, onClickFunctions)
+            };
+        }
+
+        const referenceComponent = React.cloneElement(control, controlProps);
 
         const popoverClasses = classnames('fd-popover', className);
 
@@ -88,6 +127,7 @@ Popover.propTypes = {
     customStyles: PropTypes.object,
     disabled: PropTypes.bool,
     disableEdgeDetection: PropTypes.bool,
+    disableKeyPressHandler: PropTypes.bool,
     disableStyles: PropTypes.bool,
     noArrow: PropTypes.bool,
     placement: PropTypes.oneOf(POPPER_PLACEMENTS),
@@ -105,6 +145,7 @@ Popover.propDescriptions = {
     body: 'Node(s) to render in the overlay.',
     control: 'Node to render as the reference element (that the `body` will be placed in relation to).',
     disableEdgeDetection: 'Set to **true** to render popover without edge detection so popover will not flip from top to bottom with scroll.',
+    disableKeyPressHandler: 'Set to **true** to remove onKeyPress handler and aria-* roles. Only do so if the control is a complex component such as a FormInput with Button.',
     noArrow: 'Set to **true** to render a popover without an arrow.',
     placement: 'Initial position of the `body` (overlay) related to the `control`.',
     popperProps: 'Additional props to be spread to the overlay element, supported by <a href="https://popper.js.org" target="_blank">popper.js</a>.',
diff --git a/src/Popover/Popover.test.js b/src/Popover/Popover.test.js
@@ -182,6 +182,53 @@ describe('<Popover />', () => {
         });
     });
 
+    describe('control accessibility', () => {
+        test('adds a tabindex of 0 to the control', () => {
+            const wrapper = mountComponentWithStyles(popOver);
+            const button = wrapper.find('Icon').at(0);
+
+            expect(button.props().tabIndex).toEqual(0);
+        });
+
+        test('adds aria-haspopup to the control', () => {
+            const wrapper = mountComponentWithStyles(popOver);
+            const button = wrapper.find('Icon').at(0);
+
+            expect(button.props()['aria-haspopup']).toEqual(true);
+        });
+
+        test('adds a role of button to the control', () => {
+            const wrapper = mountComponentWithStyles(popOver);
+            const button = wrapper.find('Icon').at(0);
+
+            expect(button.props().role).toEqual('button');
+        });
+
+        test('handle space key to open popover', () => {
+            const syntheticEvent = {
+                keyCode: 32,
+                preventDefault: () => {}
+            };
+            const wrapper = mountComponentWithStyles(popOver);
+            const button = wrapper.find('Icon').at(0);
+            button.prop('onKeyPress')(syntheticEvent, 'Icon', wrapper.triggerBody);
+
+            expect(wrapper.state('isExpanded')).toBeTruthy();
+        });
+
+        test('handle enter key to open popover', () => {
+            const syntheticEvent = {
+                keyCode: 13,
+                preventDefault: () => {}
+            };
+            const wrapper = mountComponentWithStyles(popOver);
+            const button = wrapper.find('Icon').at(0);
+            button.prop('onKeyPress')(syntheticEvent, 'Icon', wrapper.triggerBody);
+
+            expect(wrapper.state('isExpanded')).toBeTruthy();
+        });
+    });
+
     describe('Callback handler', () => {
         test('should dispatch the onClickOutside callback with the event', () => {
             let f = jest.fn();
diff --git a/src/Popover/__snapshots__/Popover.test.js.snap b/src/Popover/__snapshots__/Popover.test.js.snap
@@ -13,8 +13,12 @@ exports[`<Popover /> create Popover 1`] = `
       className="fd-popover__control"
     >
       <span
+        aria-haspopup={true}
         className="sap-icon--cart sap-icon--xl"
         onClick={[Function]}
+        onKeyPress={[Function]}
+        role="button"
+        tabIndex={0}
       />
     </div>
   </div>
@@ -34,8 +38,12 @@ exports[`<Popover /> create Popover 2`] = `
       className="fd-popover__control"
     >
       <span
+        aria-haspopup={true}
         className="sap-icon--cart sap-icon--xl"
         onClick={[Function]}
+        onKeyPress={[Function]}
+        role="button"
+        tabIndex={0}
       />
     </div>
   </div>
@@ -55,8 +63,12 @@ exports[`<Popover /> create Popover 3`] = `
       className="fd-popover__control"
     >
       <span
+        aria-haspopup={true}
         className="sap-icon--cart sap-icon--xl"
         onClick={[Function]}
+        onKeyPress={[Function]}
+        role="button"
+        tabIndex={0}
       />
     </div>
   </div>
@@ -76,8 +88,12 @@ exports[`<Popover /> create Popover 4`] = `
       className="fd-popover__control"
     >
       <span
+        aria-haspopup={true}
         className="sap-icon--cart sap-icon--xl"
         onClick={[Function]}
+        onKeyPress={[Function]}
+        role="button"
+        tabIndex={0}
       />
     </div>
   </div>
diff --git a/src/SearchInput/SearchInput.js b/src/SearchInput/SearchInput.js
@@ -184,6 +184,7 @@ class SearchInput extends Component {
                             )}
                         </div>
                     }
+                    disableKeyPressHandler
                     disableStyles={disableStyles} />
             </div>
         );
diff --git a/src/Shellbar/__snapshots__/Shellbar.test.js.snap b/src/Shellbar/__snapshots__/Shellbar.test.js.snap
diff --git a/src/Tile/__snapshots__/Tile.test.js.snap b/src/Tile/__snapshots__/Tile.test.js.snap
diff --git a/src/Tile/__snapshots__/TileActions.test.js.snap b/src/Tile/__snapshots__/TileActions.test.js.snap

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ const ComboboxInput = React.forwardRef(({ placeholder, menu, compact, className,`
`37`	`37`	`</span>`
`38`	`38`	`</div>`
`39`	`39`	`}`
	`40`	`+ disableKeyPressHandler`
`40`	`41`	`disableStyles={disableStyles}`
`41`	`42`	`noArrow />`
`42`	`43`	`</div>`
Original file line number	Diff line number	Diff line change
`@@ -110,6 +110,7 @@ const LocalizationEditor = React.forwardRef(({ control, menu, id, compact, texta`
`110`	`110`	`</span>`
`111`	`111`	`</div>`
`112`	`112`	`}`
	`113`	`+ disableKeyPressHandler`
`113`	`114`	`disableStyles={disableStyles}`
`114`	`115`	`id={id}`
`115`	`116`	`noArrow />`
Original file line number	Diff line number	Diff line change
`@@ -184,6 +184,7 @@ class SearchInput extends Component {`
`184`	`184`	`)}`
`185`	`185`	`</div>`
`186`	`186`	`}`
	`187`	`+ disableKeyPressHandler`
`187`	`188`	`disableStyles={disableStyles} />`
`188`	`189`	`</div>`
`189`	`190`	`);`