refactor(Dropdown and ApplicationLauncher): Refactor keyHandler usage (…

…#1382) * refactor(Dropdown and ApplicationLauncher): Refactor keyHandler so it can be shared by dropdown-like Fixes #1374 * refactor(ApplicationLauncher and Dropdown): Add tests and comments and refactored constants Fixes #1374
patternfly · Feb 14, 2019 · 1743ad6 · 1743ad6
1 parent 8e1b471
commit 1743ad6
Show file tree

Hide file tree

Showing 7 changed files with 175 additions and 95 deletions.
diff --git a/...ges/patternfly-4/react-core/src/components/ApplicationLauncher/ApplicationLauncherMenu.js b/...ges/patternfly-4/react-core/src/components/ApplicationLauncher/ApplicationLauncherMenu.js
@@ -5,6 +5,8 @@ import PropTypes from 'prop-types';
 import { ApplicationLauncherPosition } from './applicationLauncherConstants';
 import { DropdownContext, DropdownArrowContext } from '../Dropdown/dropdownConstants';
 import ReactDOM from 'react-dom';
+import { keyHandler } from '../../helpers/util';
+import { KEY_CODES, KEYHANDLER_DIRECTION } from '../../helpers/constants';
 
 const propTypes = {
   /** Anything which can be rendered as dropdown items */
@@ -27,46 +29,22 @@ const defaultProps = {
 };
 
 class ApplicationLauncherMenu extends React.Component {
-  refsCollection = {};
+  refsCollection = [];
 
-  keyHandler = (index, position, custom = false) => {
-    const kids = this.props.children;
-    if (!Array.isArray(kids)) return;
-    let nextIndex;
-    if (position === 'up') {
-      if (index === 0) {
-        // loop back to end
-        nextIndex = kids.length - 1;
-      } else {
-        nextIndex = index - 1;
-      }
-    } else if (index === kids.length - 1) {
-      // loop back to beginning
-      nextIndex = 0;
-    } else {
-      nextIndex = index + 1;
-    }
-    if (this.refsCollection[`item-${nextIndex}`] === null) {
-      this.keyHandler(nextIndex, position, custom);
-    } else {
-      custom
-        ? (this.refsCollection[`item-${nextIndex}`].current.focus &&
-            this.refsCollection[`item-${nextIndex}`].current.focus()) ||
-          ReactDOM.findDOMNode(this.refsCollection[`item-${nextIndex}`].current).focus()
-        : this.refsCollection[`item-${nextIndex}`].focus();
-    }
+  childKeyHandler = (index, position, custom = false) => {
+    keyHandler(index, position, this.refsCollection, this.props.children, custom);
   };
 
   onKeyDown = event => {
     // Detected key press on this item, notify the menu parent so that the appropriate
     // item can be focused
-    if (event.key === 'Tab') return;
+    if (event.key === KEY_CODES.TAB) return;
     event.preventDefault();
-    if (event.key === 'ArrowUp') {
-      this.keyHandler(this.props.index, 'up');
-    } else if (event.key === 'ArrowDown') {
-      this.keyHandler(this.props.index, 'down');
-    } else if (event.key === 'Enter') {
+    if (event.keyCode === KEY_CODES.ARROW_UP) {
+      keyHandler(this.props.index, KEYHANDLER_DIRECTION.UP, this.refsCollection, this.props.children);
+    } else if (event.keyCode === KEY_CODES.ARROW_DOWN) {
+      keyHandler(this.props.index, KEYHANDLER_DIRECTION.DOWN, this.refsCollection, this.props.children);
+    } else if (event.key === KEY_CODES.ENTER) {
       if (!this.ref.current.getAttribute) ReactDOM.findDOMNode(this.ref.current).click();
       else {
         this.ref.current.click && this.ref.current.click();
@@ -76,17 +54,17 @@ class ApplicationLauncherMenu extends React.Component {
 
   componentDidMount() {
     if (this.props.openedOnEnter) {
-      this.refsCollection['item-0'].focus();
+      this.refsCollection[0].focus();
     }
   }
 
   sendRef = (index, node, isDisabled) => {
     if (!node.getAttribute) {
-      this.refsCollection[`item-${index}`] = ReactDOM.findDOMNode(node);
+      this.refsCollection[index] = ReactDOM.findDOMNode(node);
     } else if (isDisabled || node.getAttribute('role') === 'separator') {
-      this.refsCollection[`item-${index}`] = null;
+      this.refsCollection[index] = null;
     } else {
-      this.refsCollection[`item-${index}`] = node;
+      this.refsCollection[index] = node;
     }
   };
 
@@ -104,7 +82,7 @@ class ApplicationLauncherMenu extends React.Component {
     return (
       <DropdownArrowContext.Provider
         value={{
-          keyHandler: this.keyHandler,
+          keyHandler: this.childKeyHandler,
           sendRef: this.sendRef
         }}
       >

diff --git a/packages/patternfly-4/react-core/src/components/ApplicationLauncher/Toggle.js b/packages/patternfly-4/react-core/src/components/ApplicationLauncher/Toggle.js
@@ -64,18 +64,18 @@ class ApplicationLauncherToggle extends Component {
   onEscPress = event => {
     const { parentRef } = this.props;
     const keyCode = event.keyCode || event.which;
-    if (this.props.isOpen && (keyCode === KEY_CODES.ESCAPE_KEY || event.key === 'Tab') && parentRef && parentRef.contains(event.target)) {
+    if (this.props.isOpen && (keyCode === KEY_CODES.ESCAPE_KEY || event.keyCode === KEY_CODES.TAB) && parentRef && parentRef.contains(event.target)) {
       this.props.onToggle && this.props.onToggle(false);
       this.toggle.focus();
     }
   };
 
   onKeyDown = event => {
-    if (event.key === 'Tab' && !this.props.isOpen) return;
+    if (event.keyCode === KEY_CODES.TAB && !this.props.isOpen) return;
     event.preventDefault();
-    if ((event.key === 'Tab' || event.key === 'Enter' || event.key === ' ') && this.props.isOpen) {
+    if ((event.keyCode === KEY_CODES.TAB || event.keyCode === KEY_CODES.ENTER || event.key === ' ') && this.props.isOpen) {
       this.props.onToggle(!this.props.isOpen);
-    } else if ((event.key === 'Enter' || event.key === ' ') && !this.props.isOpen) {
+    } else if ((event.keyCode === KEY_CODES.ENTER || event.key === ' ') && !this.props.isOpen) {
       this.props.onToggle(!this.props.isOpen);
       this.props.onEnter();
     }

diff --git a/packages/patternfly-4/react-core/src/components/Dropdown/DropdownItem.js b/packages/patternfly-4/react-core/src/components/Dropdown/DropdownItem.js
@@ -6,6 +6,7 @@ import { css } from '@patternfly/react-styles';
 import PropTypes from 'prop-types';
 import { componentShape } from '../../helpers/componentShape';
 import { DropdownContext } from './dropdownConstants';
+import { KEY_CODES, KEYHANDLER_DIRECTION } from '../../helpers/constants';
 
 const propTypes = {
   /** Anything which can be rendered as dropdown item */
@@ -56,13 +57,13 @@ class DropdownItem extends React.Component {
   onKeyDown = event => {
     // Detected key press on this item, notify the menu parent so that the appropriate
     // item can be focused
-    if (event.key === 'Tab') return;
+    if (event.keyCode === KEY_CODES.TAB) return;
     event.preventDefault();
-    if (event.key === 'ArrowUp') {
-      this.props.context.keyHandler(this.props.index, 'up');
-    } else if (event.key === 'ArrowDown') {
-      this.props.context.keyHandler(this.props.index, 'down');
-    } else if (event.key === 'Enter') {
+    if (event.keyCode === KEY_CODES.ARROW_UP) {
+      this.props.context.keyHandler(this.props.index, KEYHANDLER_DIRECTION.UP);
+    } else if (event.keyCode === KEY_CODES.ARROW_DOWN) {
+      this.props.context.keyHandler(this.props.index, KEYHANDLER_DIRECTION.DOWN);
+    } else if (event.keyCode === KEY_CODES.ENTER) {
       if (!this.ref.current.getAttribute) ReactDOM.findDOMNode(this.ref.current).click();
       else {
         this.ref.current.click && this.ref.current.click();

diff --git a/packages/patternfly-4/react-core/src/components/Dropdown/DropdownMenu.js b/packages/patternfly-4/react-core/src/components/Dropdown/DropdownMenu.js
@@ -3,8 +3,10 @@ import styles from '@patternfly/patternfly/components/Dropdown/dropdown.css';
 import { css } from '@patternfly/react-styles';
 import PropTypes from 'prop-types';
 import { componentShape } from '../../helpers/componentShape';
-import { DropdownPosition, DropdownContext, DropdownArrowContext } from './dropdownConstants';
 import ReactDOM from 'react-dom';
+import { keyHandler } from '../../helpers/util';
+import { DropdownPosition, DropdownDirection, DropdownArrowContext, DropdownContext } from './dropdownConstants';
+import { KEY_CODES, KEYHANDLER_DIRECTION } from '../../helpers/constants';
 
 const propTypes = {
   /** Anything which can be rendered as dropdown items */
@@ -30,53 +32,29 @@ const defaultProps = {
 };
 
 class DropdownMenu extends React.Component {
-  refsCollection = {};
-
-  keyHandler = (index, position, custom = false) => {
-    const kids = this.props.children;
-    if (!Array.isArray(kids)) return;
-    let nextIndex;
-    if (position === 'up') {
-      if (index === 0) {
-        // loop back to end
-        nextIndex = kids.length - 1;
-      } else {
-        nextIndex = index - 1;
-      }
-    } else if (index === kids.length - 1) {
-      // loop back to beginning
-      nextIndex = 0;
-    } else {
-      nextIndex = index + 1;
-    }
-    if (this.refsCollection[`item-${nextIndex}`] === null) {
-      this.keyHandler(nextIndex, position, custom);
-    } else {
-      custom
-        ? (this.refsCollection[`item-${nextIndex}`].current.focus &&
-            this.refsCollection[`item-${nextIndex}`].current.focus()) ||
-          ReactDOM.findDOMNode(this.refsCollection[`item-${nextIndex}`].current).focus()
-        : this.refsCollection[`item-${nextIndex}`].focus();
-    }
-  };
+  refsCollection = [];
 
   componentDidMount() {
     if (this.props.openedOnEnter) {
-      if (this.props.component === 'ul') this.refsCollection['item-0'].focus();
+      if (this.props.component === 'ul') this.refsCollection[0].focus();
       else {
-        (this.refsCollection['item-0'].current.focus && this.refsCollection['item-0'].current.focus()) ||
-          ReactDOM.findDOMNode(this.refsCollection['item-0'].current).focus();
+        (this.refsCollection[0].current.focus && this.refsCollection[0].current.focus()) ||
+          ReactDOM.findDOMNode(this.refsCollection[0].current).focus();
       }
     }
   }
 
+  childKeyHandler = (index, position, custom = false) => {
+    keyHandler(index, position, this.refsCollection, this.props.children, custom);
+  };
+
   sendRef = (index, node, isDisabled) => {
     if (!node.getAttribute) {
-      this.refsCollection[`item-${index}`] = ReactDOM.findDOMNode(node);
+      this.refsCollection[index] = ReactDOM.findDOMNode(node);
     } else if (isDisabled || node.getAttribute('role') === 'separator') {
-      this.refsCollection[`item-${index}`] = null;
+      this.refsCollection[index] = null;
     } else {
-      this.refsCollection[`item-${index}`] = node;
+      this.refsCollection[index] = node;
     }
   };
 
@@ -99,18 +77,18 @@ class DropdownMenu extends React.Component {
         )}${child.props.className ? child.props.className : ''}`,
         tabIndex: -1,
         onKeyDown: event => {
-          if (event.key === 'Tab') return;
+          if (event.keyCode === KEY_CODES.TAB) return;
           event.preventDefault();
-          if (event.key === 'ArrowUp') {
-            this.keyHandler(index, 'up', true);
-          } else if (event.key === 'ArrowDown') {
-            this.keyHandler(index, 'down', true);
+          if (event.keyCode === KEY_CODES.ARROW_UP) {
+            keyHandler(index, KEYHANDLER_DIRECTION.up, this.refsCollection, this.props.children, true);
+          } else if (event.keyCode === KEY_CODES.ARROW_DOWN) {
+            keyHandler(index, KEYHANDLER_DIRECTION.DOWN, this.refsCollection, this.props.children, true);
           }
         }
       });
       !mappedChild.props.disabled
-        ? (this.refsCollection[`item-${index}`] = mappedChild.ref)
-        : (this.refsCollection[`item-${index}`] = null);
+        ? (this.refsCollection[index] = mappedChild.ref)
+        : (this.refsCollection[index] = null);
       return mappedChild;
     });
     return mappedChildren;
@@ -122,7 +100,7 @@ class DropdownMenu extends React.Component {
     return (
       <DropdownArrowContext.Provider
         value={{
-          keyHandler: this.keyHandler,
+          keyHandler: this.childKeyHandler,
           sendRef: this.sendRef
         }}
       >

diff --git a/packages/patternfly-4/react-core/src/helpers/constants.ts b/packages/patternfly-4/react-core/src/helpers/constants.ts
@@ -1,3 +1,5 @@
-export const KEY_CODES = { ESCAPE_KEY: 27 };
+export const KEY_CODES = { ARROW_UP: 38, ARROW_DOWN: 40, ESCAPE_KEY: 27, TAB: 9, ENTER: 13 };
 
 export const SIDE = { RIGHT: 'right', LEFT: 'left', BOTH: 'both', NONE: 'none' };
+
+export const KEYHANDLER_DIRECTION = { UP: 'up', DOWN: 'down'};
diff --git a/packages/patternfly-4/react-core/src/helpers/util.test.js b/packages/patternfly-4/react-core/src/helpers/util.test.js
@@ -1,5 +1,8 @@
+import React from 'react';
+import { mount } from 'enzyme';
+import { ApplicationLauncher, DropdownItem, Dropdown, DropdownToggle } from '@patternfly/react-core';
 import { capitalize, getUniqueId, debounce, isElementInView, sideElementIsOutOfView } from './util';
-import { SIDE } from './constants';
+import { KEY_CODES, SIDE } from './constants';
 
 test('capitalize', () => {
   expect(capitalize('foo')).toBe('Foo');
@@ -72,3 +75,82 @@ test('sideElementIsOutOfView Returns NONE when in view', () => {
   const element = { offsetLeft: 10, clientWidth: 100 };
   expect(sideElementIsOutOfView(container, element)).toBe(SIDE.NONE);
 });
+
+describe('keyHandler works on ApplicationLauncher', () => {
+  document.body.innerHTML = '<!doctype html><html><body></body></html>';
+  const dropdownItems = [
+    <DropdownItem key="link" id="first">Link</DropdownItem>,
+    <DropdownItem key="action" id="second" component="button">
+      Action
+    </DropdownItem>,
+    <DropdownItem key="disabled link" id="third" isDisabled>
+      Disabled Link
+    </DropdownItem>,
+  ];
+  const view = mount(<ApplicationLauncher dropdownItems={dropdownItems} isOpen />, {
+    attachTo: document.getElementsByName('div')[0]
+  });
+  const firstDropdownItem = view.find('#first').first();
+  const secondDropdownItem = view.find('#second').first();
+  const thirdDropdownItem = view.find('#third').first();
+
+  test('keyHandler advances forward', () => {
+    firstDropdownItem.simulate('keydown', { key: 'ArrowDown', keyCode: KEY_CODES.ARROW_DOWN, which: KEY_CODES.ARROW_DOWN });
+    expect(secondDropdownItem === document.activeElement);
+  });
+
+  test('keyHandler regresses backward', () => {
+    secondDropdownItem.simulate('keydown', { key: 'ArrowUp', keyCode: KEY_CODES.ARROW_UP, which: KEY_CODES.ARROW_UP });
+    expect(firstDropdownItem === document.activeElement);
+  });
+
+  test('keyHandler skips disabled items and loops down to top', () => {
+    secondDropdownItem.simulate('keydown', { key: 'ArrowDown', keyCode: KEY_CODES.ARROW_DOWN, which: KEY_CODES.ARROW_DOWN });
+    expect(firstDropdownItem === document.activeElement);
+  });
+
+   test('keyHandler loops top to bottom', () => {
+    firstDropdownItem.simulate('keydown', { key: 'ArrowUp', keyCode: KEY_CODES.ARROW_UP, which: KEY_CODES.ARROW_UP });
+    expect(secondDropdownItem === document.activeElement);
+  });
+});
+
+describe('keyHandler works on Dropdown', () => {
+  document.body.innerHTML = '<!doctype html><html><body></body></html>';
+  const dropdownItems = [
+    <DropdownItem key="link" id="first">Link</DropdownItem>,
+    <DropdownItem key="action" id="second" component="button">
+      Action
+    </DropdownItem>,
+    <DropdownItem key="disabled link" id="third" isDisabled>
+      Disabled Link
+    </DropdownItem>,
+  ];
+  const view = mount(<Dropdown dropdownItems={dropdownItems} isOpen toggle={<DropdownToggle>Expanded Dropdown</DropdownToggle>} />, {
+    attachTo: document.getElementsByName('div')[0]
+  });
+  const firstDropdownItem = view.find('#first').first();
+  const secondDropdownItem = view.find('#second').first();
+  const thirdDropdownItem = view.find('#third').first();
+
+  test('keyHandler advances forward', () => {
+    firstDropdownItem.simulate('keydown', { key: 'ArrowDown', keyCode: KEY_CODES.ARROW_DOWN, which: KEY_CODES.ARROW_DOWN });
+    expect(secondDropdownItem === document.activeElement);
+  });
+
+  test('keyHandler regresses backward', () => {
+    secondDropdownItem.simulate('keydown', { key: 'ArrowUp', keyCode: KEY_CODES.ARROW_UP, which: KEY_CODES.ARROW_UP });
+    expect(firstDropdownItem === document.activeElement);
+  });
+
+  test('keyHandler skips disabled items and loops down to top', () => {
+    secondDropdownItem.simulate('keydown', { key: 'ArrowDown', keyCode: KEY_CODES.ARROW_DOWN, which: KEY_CODES.ARROW_DOWN });
+    expect(firstDropdownItem === document.activeElement);
+  });
+
+   test('keyHandler loops top to bottom', () => {
+    firstDropdownItem.simulate('keydown', { key: 'ArrowUp', keyCode: KEY_CODES.ARROW_UP, which: KEY_CODES.ARROW_UP });
+    expect(secondDropdownItem === document.activeElement);
+  });
+});
+