From 506023b0f3cc7dff21a298ee9573d6426d86d386 Mon Sep 17 00:00:00 2001
From: Mark McDowall <mark@mcdowall.ca>
Date: Sun, 1 Mar 2020 21:03:38 -0800
Subject: [PATCH] Scrolling and hotkey improvements

New: Use Esc/Enter for cancel/accept in confirmation modals
Fixed: Modals focused when opened
Fixed: Scrolling with keyboard unless focus is shifted out of scrollable area
Closes #3291
---
 frontend/src/Components/Modal/ConfirmModal.js | 17 +++++--
 frontend/src/Components/Modal/Modal.js        | 39 ++++++++-------
 frontend/src/Components/Scroller/Scroller.js  |  3 ++
 frontend/src/Components/keyboardShortcuts.js  | 10 ++++
 frontend/src/Settings/PendingChangesModal.js  | 16 ++++--
 package.json                                  |  1 +
 yarn.lock                                     | 49 +++++++++++++++++++
 7 files changed, 108 insertions(+), 27 deletions(-)

diff --git a/frontend/src/Components/Modal/ConfirmModal.js b/frontend/src/Components/Modal/ConfirmModal.js
index 5bb783d43..529a5d973 100644
--- a/frontend/src/Components/Modal/ConfirmModal.js
+++ b/frontend/src/Components/Modal/ConfirmModal.js
@@ -1,6 +1,7 @@
 import PropTypes from 'prop-types';
-import React from 'react';
+import React, { useEffect } from 'react';
 import { kinds, sizes } from 'Helpers/Props';
+import keyboardShortcuts from 'Components/keyboardShortcuts';
 import Button from 'Components/Link/Button';
 import SpinnerButton from 'Components/Link/SpinnerButton';
 import Modal from 'Components/Modal/Modal';
@@ -21,9 +22,14 @@ function ConfirmModal(props) {
     hideCancelButton,
     isSpinning,
     onConfirm,
-    onCancel
+    onCancel,
+    bindShortcut
   } = props;
 
+  useEffect(() => {
+    bindShortcut('enter', onConfirm);
+  }, [onConfirm]);
+
   return (
     <Modal
       isOpen={isOpen}
@@ -49,7 +55,7 @@ function ConfirmModal(props) {
           }
 
           <SpinnerButton
-            data-autofocus={true}
+            autoFocus={true}
             kind={kind}
             isSpinning={isSpinning}
             onPress={onConfirm}
@@ -74,7 +80,8 @@ ConfirmModal.propTypes = {
   hideCancelButton: PropTypes.bool,
   isSpinning: PropTypes.bool.isRequired,
   onConfirm: PropTypes.func.isRequired,
-  onCancel: PropTypes.func.isRequired
+  onCancel: PropTypes.func.isRequired,
+  bindShortcut: PropTypes.func.isRequired
 };
 
 ConfirmModal.defaultProps = {
@@ -85,4 +92,4 @@ ConfirmModal.defaultProps = {
   isSpinning: false
 };
 
-export default ConfirmModal;
+export default keyboardShortcuts(ConfirmModal);
diff --git a/frontend/src/Components/Modal/Modal.js b/frontend/src/Components/Modal/Modal.js
index 3b6485ce3..bb0f6c199 100644
--- a/frontend/src/Components/Modal/Modal.js
+++ b/frontend/src/Components/Modal/Modal.js
@@ -1,6 +1,7 @@
 import PropTypes from 'prop-types';
 import React, { Component } from 'react';
 import ReactDOM from 'react-dom';
+import FocusLock from 'react-focus-lock';
 import classNames from 'classnames';
 import elementClass from 'element-class';
 import getUniqueElememtId from 'Utilities/getUniqueElementId';
@@ -181,31 +182,33 @@ class Modal extends Component {
     }
 
     return ReactDOM.createPortal(
-      <div
-        className={styles.modalContainer}
-      >
+      <FocusLock disabled={false}>
         <div
-          ref={this._setBackgroundRef}
-          className={backdropClassName}
-          onMouseDown={this.onBackdropBeginPress}
-          onMouseUp={this.onBackdropEndPress}
+          className={styles.modalContainer}
         >
           <div
-            className={classNames(
-              className,
-              styles[size]
-            )}
-            style={style}
+            ref={this._setBackgroundRef}
+            className={backdropClassName}
+            onMouseDown={this.onBackdropBeginPress}
+            onMouseUp={this.onBackdropEndPress}
           >
-            <ErrorBoundary
-              errorComponent={ModalError}
-              onModalClose={onModalClose}
+            <div
+              className={classNames(
+                className,
+                styles[size]
+              )}
+              style={style}
             >
-              {children}
-            </ErrorBoundary>
+              <ErrorBoundary
+                errorComponent={ModalError}
+                onModalClose={onModalClose}
+              >
+                {children}
+              </ErrorBoundary>
+            </div>
           </div>
         </div>
-      </div>,
+      </FocusLock>,
       this._node
     );
   }
diff --git a/frontend/src/Components/Scroller/Scroller.js b/frontend/src/Components/Scroller/Scroller.js
index 22fae04c6..4ae94c78b 100644
--- a/frontend/src/Components/Scroller/Scroller.js
+++ b/frontend/src/Components/Scroller/Scroller.js
@@ -23,6 +23,8 @@ class Scroller extends Component {
     if (this.props.scrollTop != null) {
       this._scroller.scrollTop = scrollTop;
     }
+
+    this._scroller.focus({ preventScroll: true });
   }
 
   //
@@ -58,6 +60,7 @@ class Scroller extends Component {
           styles[scrollDirection],
           autoScroll && styles.autoScroll
         )}
+        tabIndex={-1}
         {...otherProps}
       >
         {children}
diff --git a/frontend/src/Components/keyboardShortcuts.js b/frontend/src/Components/keyboardShortcuts.js
index f43139e4a..ab33a378f 100644
--- a/frontend/src/Components/keyboardShortcuts.js
+++ b/frontend/src/Components/keyboardShortcuts.js
@@ -8,6 +8,16 @@ export const shortcuts = {
     name: 'Open This Modal'
   },
 
+  CLOSE_MODAL: {
+    key: 'Esc',
+    name: 'Close Current Modal'
+  },
+
+  ACCEPT_CONFIRM_MODAL: {
+    key: 'Enter',
+    name: 'Accept Confirmation Modal'
+  },
+
   SERIES_SEARCH_INPUT: {
     key: 's',
     name: 'Focus Search Box'
diff --git a/frontend/src/Settings/PendingChangesModal.js b/frontend/src/Settings/PendingChangesModal.js
index e3b14e228..b23d86418 100644
--- a/frontend/src/Settings/PendingChangesModal.js
+++ b/frontend/src/Settings/PendingChangesModal.js
@@ -1,6 +1,7 @@
 import PropTypes from 'prop-types';
-import React from 'react';
+import React, { useEffect } from 'react';
 import { kinds } from 'Helpers/Props';
+import keyboardShortcuts from 'Components/keyboardShortcuts';
 import Button from 'Components/Link/Button';
 import Modal from 'Components/Modal/Modal';
 import ModalContent from 'Components/Modal/ModalContent';
@@ -12,9 +13,14 @@ function PendingChangesModal(props) {
   const {
     isOpen,
     onConfirm,
-    onCancel
+    onCancel,
+    bindShortcut
   } = props;
 
+  useEffect(() => {
+    bindShortcut('enter', onConfirm);
+  }, [onConfirm]);
+
   return (
     <Modal
       isOpen={isOpen}
@@ -36,6 +42,7 @@ function PendingChangesModal(props) {
           </Button>
 
           <Button
+            autoFocus={true}
             kind={kinds.DANGER}
             onPress={onConfirm}
           >
@@ -52,11 +59,12 @@ PendingChangesModal.propTypes = {
   isOpen: PropTypes.bool.isRequired,
   kind: PropTypes.oneOf(kinds.all),
   onConfirm: PropTypes.func.isRequired,
-  onCancel: PropTypes.func.isRequired
+  onCancel: PropTypes.func.isRequired,
+  bindShortcut: PropTypes.func.isRequired
 };
 
 PendingChangesModal.defaultProps = {
   kind: kinds.PRIMARY
 };
 
-export default PendingChangesModal;
+export default keyboardShortcuts(PendingChangesModal);
diff --git a/package.json b/package.json
index b5e9dcd8b..6e77a0d6e 100644
--- a/package.json
+++ b/package.json
@@ -96,6 +96,7 @@
     "react-dnd-html5-backend": "9.3.2",
     "react-document-title": "2.0.3",
     "react-dom": "16.8.6",
+    "react-focus-lock": "2.2.1",
     "react-google-recaptcha": "1.1.0",
     "react-lazyload": "2.6.2",
     "react-measure": "1.4.7",
diff --git a/yarn.lock b/yarn.lock
index 255e28f55..e43b1f221 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -825,6 +825,13 @@
     "@babel/plugin-transform-react-jsx-self" "^7.0.0"
     "@babel/plugin-transform-react-jsx-source" "^7.0.0"
 
+"@babel/runtime@^7.0.0":
+  version "7.8.4"
+  resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.8.4.tgz#d79f5a2040f7caa24d53e563aad49cbc05581308"
+  integrity sha512-neAp3zt80trRVBI1x0azq6c57aNBqYZH8KhMm3TaB7wEI5Q4A2SHfBHE8w9gOhI/lrqxtEbXZgQIrHP+wvSGwQ==
+  dependencies:
+    regenerator-runtime "^0.13.2"
+
 "@babel/runtime@^7.1.2", "@babel/runtime@^7.4.0", "@babel/runtime@^7.4.5":
   version "7.5.5"
   resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.5.5.tgz#74fba56d35efbeca444091c7850ccd494fd2f132"
@@ -2857,6 +2864,11 @@ detect-newline@2.X:
   resolved "https://registry.yarnpkg.com/detect-newline/-/detect-newline-2.1.0.tgz#f41f1c10be4b00e87b5f13da680759f2c5bfd3e2"
   integrity sha1-9B8cEL5LAOh7XxPaaAdZ8sW/0+I=
 
+detect-node@^2.0.4:
+  version "2.0.4"
+  resolved "https://registry.yarnpkg.com/detect-node/-/detect-node-2.0.4.tgz#014ee8f8f669c5c58023da64b8179c083a28c46c"
+  integrity sha512-ZIzRpLJrOj7jjP2miAtgqIfmzbxa4ZOr5jJc601zklsfEx9oTzmmj2nVpIPRpNlRTIh8lc1kyViIY7BWSGNmKw==
+
 diff@^1.3.2:
   version "1.4.0"
   resolved "https://registry.yarnpkg.com/diff/-/diff-1.4.0.tgz#7f28d2eb9ee7b15a97efd89ce63dcfdaa3ccbabf"
@@ -3817,6 +3829,11 @@ flush-write-stream@^1.0.0, flush-write-stream@^1.0.2:
     inherits "^2.0.3"
     readable-stream "^2.3.6"
 
+focus-lock@^0.6.6:
+  version "0.6.6"
+  resolved "https://registry.yarnpkg.com/focus-lock/-/focus-lock-0.6.6.tgz#98119a755a38cfdbeda0280eaa77e307eee850c7"
+  integrity sha512-Dx69IXGCq1qsUExWuG+5wkiMqVM/zGx/reXSJSLogECwp3x6KeNQZ+NAetgxEFpnC41rD8U3+jRCW68+LNzdtw==
+
 for-in@^1.0.1, for-in@^1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/for-in/-/for-in-1.0.2.tgz#81068d295a8142ec0ac726c6e2200c30fb6d5e80"
@@ -7268,6 +7285,13 @@ react-autowhatever@^10.1.2:
     react-themeable "^1.1.0"
     section-iterator "^2.0.0"
 
+react-clientside-effect@^1.2.2:
+  version "1.2.2"
+  resolved "https://registry.yarnpkg.com/react-clientside-effect/-/react-clientside-effect-1.2.2.tgz#6212fb0e07b204e714581dd51992603d1accc837"
+  integrity sha512-nRmoyxeok5PBO6ytPvSjKp9xwXg9xagoTK1mMjwnQxqM9Hd7MNPl+LS1bOSOe+CV2+4fnEquc7H/S8QD3q697A==
+  dependencies:
+    "@babel/runtime" "^7.0.0"
+
 react-custom-scrollbars@4.2.1:
   version "4.2.1"
   resolved "https://registry.yarnpkg.com/react-custom-scrollbars/-/react-custom-scrollbars-4.2.1.tgz#830fd9502927e97e8a78c2086813899b2a8b66db"
@@ -7313,6 +7337,18 @@ react-dom@16.8.6:
     prop-types "^15.6.2"
     scheduler "^0.13.6"
 
+react-focus-lock@2.2.1:
+  version "2.2.1"
+  resolved "https://registry.yarnpkg.com/react-focus-lock/-/react-focus-lock-2.2.1.tgz#1d12887416925dc53481914b7cedd39494a3b24a"
+  integrity sha512-47g0xYcCTZccdzKRGufepY8oZ3W1Qg+2hn6u9SHZ0zUB6uz/4K4xJe7yYFNZ1qT6m+2JDm82F6QgKeBTbjW4PQ==
+  dependencies:
+    "@babel/runtime" "^7.0.0"
+    focus-lock "^0.6.6"
+    prop-types "^15.6.2"
+    react-clientside-effect "^1.2.2"
+    use-callback-ref "^1.2.1"
+    use-sidecar "^1.0.1"
+
 react-google-recaptcha@1.1.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/react-google-recaptcha/-/react-google-recaptcha-1.1.0.tgz#f33bef3e22e8c016820e80da48d573f516bb99e8"
@@ -9210,6 +9246,19 @@ url@^0.11.0:
     punycode "1.3.2"
     querystring "0.2.0"
 
+use-callback-ref@^1.2.1:
+  version "1.2.1"
+  resolved "https://registry.yarnpkg.com/use-callback-ref/-/use-callback-ref-1.2.1.tgz#898759ccb9e14be6c7a860abafa3ffbd826c89bb"
+  integrity sha512-C3nvxh0ZpaOxs9RCnWwAJ+7bJPwQI8LHF71LzbQ3BvzH5XkdtlkMadqElGevg5bYBDFip4sAnD4m06zAKebg1w==
+
+use-sidecar@^1.0.1:
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/use-sidecar/-/use-sidecar-1.0.2.tgz#e72f582a75842f7de4ef8becd6235a4720ad8af6"
+  integrity sha512-287RZny6m5KNMTb/Kq9gmjafi7lQL0YHO1lYolU6+tY1h9+Z3uCtkJJ3OSOq3INwYf2hBryCcDh4520AhJibMA==
+  dependencies:
+    detect-node "^2.0.4"
+    tslib "^1.9.3"
+
 use@^3.1.0:
   version "3.1.1"
   resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f"