Improve a11y for modals (Automattic#950)

* Set id on root div * Introduce react-modal * Integrate react-modal * Remove focus ring from dialog * Fix modal overlay for Dark Theme * Fix About dialog in Dark Theme * Remove unneeded .focus-guard class * Rename `single` dialog prop to `multiple` Make all dialogs default to multiple = false * Remove unused `params` * Make all dialogs modal by default * Remove unneeded refs in Dialog * Remove unused element id in Dialog * Use separate `hideTitleBar` prop * Remove unused `params` in DialogRenderer * Add title (aria-label) to all dialogs * Use enums for dialog types/titles * Add aria-labels for dialog close buttons * Rename `dialog--box` to `dialog` * Move react-modal init to boot.js To avoid complication with the smoke test for App * Remove unnecessary test * Rename ReactModal to Modal * Remove `modal` prop from dialog data * Add spread to box-shadows on dialog (Light) Co-Authored-By: mirka <lena@jaguchi.com> * Add spread to box-shadows on dialog (Dark) Co-Authored-By: mirka <lena@jaguchi.com> * Fixup BEM fail in .dialog-renderer 🤦🏻‍♀️
Lonely-Mr-zhang · Oct 24, 2018 · 6ed2028 · 6ed2028
1 parent 6e05bc2
commit 6ed2028
Show file tree

Hide file tree

Showing 26 changed files with 298 additions and 397 deletions.
diff --git a/desktop/app.js b/desktop/app.js
@@ -17,6 +17,8 @@ const buildViewMenu = require('./menus/view-menu');
 const buildEditMenu = require('./menus/edit-menu');
 const { isDev } = require('./env');
 
+const DialogTypes = require('../lib/dialogs/types');
+
 require('module').globalPaths.push(path.resolve(path.join(__dirname)));
 
 module.exports = function main() {
@@ -180,11 +182,7 @@ function createMenuTemplate(settings) {
       if (focusedWindow) {
         focusedWindow.webContents.send('appCommand', {
           action: 'showDialog',
-          dialog: {
-            type: 'About',
-            modal: true,
-            single: true,
-          },
+          dialog: DialogTypes.ABOUT,
         });
       }
     },
@@ -197,11 +195,7 @@ function createMenuTemplate(settings) {
       if (focusedWindow) {
         focusedWindow.webContents.send('appCommand', {
           action: 'showDialog',
-          dialog: {
-            type: 'Settings',
-            modal: true,
-            single: true,
-          },
+          dialog: DialogTypes.SETTINGS,
         });
       }
     },

diff --git a/lib/app.jsx b/lib/app.jsx
@@ -374,7 +374,9 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
       } = this.props;
       const isMacApp = isElectronMac();
 
-      const appClasses = classNames('app', `theme-${settings.theme}`, {
+      const themeClass = `theme-${settings.theme}`;
+
+      const appClasses = classNames('app', themeClass, {
         'is-line-length-full': settings.lineLength === 'full',
         'touch-enabled': 'ontouchstart' in document.body,
       });
@@ -436,6 +438,8 @@ export const App = connect(mapStateToProps, mapDispatchToProps)(
           )}
           <DialogRenderer
             appProps={this.props}
+            themeClass={themeClass}
+            closeDialog={this.props.actions.closeDialog}
             dialogs={this.props.appState.dialogs}
             isElectron={isElectron()}
           />

diff --git a/lib/boot.js b/lib/boot.js
@@ -1,5 +1,6 @@
 import React from 'react';
 import App from './app';
+import Modal from 'react-modal';
 import getConfig from '../get-config';
 import simperium from './simperium';
 import store from './state';
@@ -101,10 +102,6 @@ client.on('unauthorized', () => {
   });
 });
 
-const app = document.createElement('div');
-
-document.body.appendChild(app);
-
 let props = {
   client: client,
   noteBucket: client.bucket('note'),
@@ -196,7 +193,9 @@ let props = {
   },
 };
 
+Modal.setAppElement('#root');
+
 render(
   React.createElement(Provider, { store }, React.createElement(App, props)),
-  app
+  document.getElementById('root')
 );
diff --git a/lib/dialog-renderer/index.jsx b/lib/dialog-renderer/index.jsx
@@ -1,50 +1,50 @@
-import React from 'react';
+import React, { Fragment } from 'react';
+import Modal from 'react-modal';
 import PropTypes from 'prop-types';
+import classNames from 'classnames';
+
 import * as Dialogs from '../dialogs';
-import { compact, concat, flowRight, map } from 'lodash';
 
 export const DialogRenderer = props => {
-  const { appProps, isElectron } = props;
-
-  const renderDialogs = () => {
-    const { dialogs } = props;
+  const { appProps, themeClass, closeDialog, dialogs, isElectron } = props;
 
-    const makeDialog = (dialog, key) => [
-      dialog.modal && (
-        <div key="overlay" className="dialogs-overlay" onClick={null} />
-      ),
-      renderDialog(dialog, key),
-    ];
-
-    return flowRight(compact, concat, map)(dialogs, makeDialog);
-  };
-
-  const renderDialog = (dialog, key) => {
-    const { params, ...dialogProps } = dialog;
+  const renderDialog = dialog => {
+    const { key, title, ...dialogProps } = dialog;
     const DialogComponent = Dialogs[dialog.type];
 
     if (DialogComponent === null) {
       throw new Error('Unknown dialog type.');
     }
 
+    const closeThisDialog = () => closeDialog({ key });
+
     return (
-      <DialogComponent
-        dialog={dialogProps}
-        {...appProps}
-        {...{ key, params, isElectron }}
-      />
+      <Modal
+        key={key}
+        className="dialog-renderer__content"
+        contentLabel={title}
+        isOpen
+        onRequestClose={closeThisDialog}
+        overlayClassName="dialog-renderer__overlay"
+        portalClassName={classNames('dialog-renderer__portal', themeClass)}
+      >
+        <DialogComponent
+          dialog={dialogProps}
+          requestClose={closeThisDialog}
+          isElectron={isElectron}
+          {...appProps}
+        />
+      </Modal>
     );
   };
 
-  if (props.dialogs.length === 0) {
-    return null;
-  }
-
-  return <div className="dialogs">{renderDialogs()}</div>;
+  return <Fragment>{dialogs.map(renderDialog)}</Fragment>;
 };
 
 DialogRenderer.propTypes = {
   appProps: PropTypes.object.isRequired,
+  themeClass: PropTypes.string,
+  closeDialog: PropTypes.func.isRequired,
   dialogs: PropTypes.array.isRequired,
   isElectron: PropTypes.bool.isRequired,
 };

diff --git a/lib/dialog-renderer/style.scss b/lib/dialog-renderer/style.scss
@@ -0,0 +1,23 @@
+.dialog-renderer__overlay {
+  position: fixed;
+  top: 0;
+  left: 0;
+  right: 0;
+  bottom: 0;
+  display: flex;
+  justify-content: center;
+  align-items: center;
+  background: rgba($white, .75);
+}
+
+.theme-dark {
+  .dialog-renderer__overlay {
+    background: rgba($gray-darkest, .75);
+  }
+}
+
+.dialog-renderer__content {
+  &:focus {
+    outline: 0;
+  }
+}
diff --git a/lib/dialog-renderer/test.js b/lib/dialog-renderer/test.js
diff --git a/lib/dialog/index.jsx b/lib/dialog/index.jsx
@@ -4,121 +4,44 @@ import classNames from 'classnames';
 
 export class Dialog extends Component {
   static propTypes = {
+    children: PropTypes.node.isRequired,
+    className: PropTypes.string,
+    hideTitleBar: PropTypes.bool,
     title: PropTypes.string,
-    onDone: PropTypes.func,
+    onDone: PropTypes.func.isRequired,
   };
 
-  componentDidMount() {
-    this.previouslyActiveElement = document.activeElement;
-    this.focusFirstInput(this.content);
-    this.startListening();
-  }
-
-  componentWillUnmount() {
-    this.stopListening();
-    if (this.previouslyActiveElement) {
-      this.previouslyActiveElement.focus();
-      this.previouslyActiveElement = null;
-    }
-  }
-
-  focusFirstInput = parent => {
-    const input = this.queryAllEnabledControls(parent)[0];
-
-    if (input) {
-      input.focus();
-    }
-  };
-
-  focusFirstInputEvent = () => this.focusFirstInput;
-
-  focusLastInput = parent => {
-    const inputs = this.queryAllEnabledControls(parent);
-    const input = inputs[inputs.length - 1];
-
-    if (input) {
-      input.focus();
-    }
-  };
-
-  focusLastInputEvent = () => this.focusLastInput();
-
-  interceptClick = event => {
-    if ('dialog' !== event.srcElement.getAttribute('role')) {
-      return;
-    }
-
-    event.preventDefault();
-    this.props.onDone();
-  };
-
-  queryAllEnabledControls = parent =>
-    (parent || this.box).querySelectorAll(
-      'button:enabled, input:enabled, textarea:enabled'
-    );
-
-  startListening = () => window.addEventListener('click', this.interceptClick);
-
-  stopListening = () =>
-    window.removeEventListener('click', this.interceptClick);
-
-  storeBox = ref => (this.box = ref);
-
-  storeContent = ref => (this.content = ref);
-
   render() {
-    const { className, title, children, onDone } = this.props;
-    const titleElementId = `dialog-title-${title}`;
+    const { className, hideTitleBar, title, children, onDone } = this.props;
 
     return (
       <div
-        className={classNames('dialog', className)}
-        role="dialog"
-        aria-labelledby={titleElementId}
+        className={classNames(
+          className,
+          'dialog theme-color-bg theme-color-fg theme-color-border'
+        )}
       >
-        <input
-          type="text"
-          className="focus-guard"
-          onFocus={this.focusLastInput}
-          spellCheck={false}
-        />
-
-        <div
-          ref={this.storeBox}
-          className="dialog-box theme-color-bg theme-color-fg theme-color-border"
-        >
-          {title &&
-            onDone && (
-              <div className="dialog-title-bar theme-color-border">
-                <div className="dialog-title-side" />
-                <h2 id={titleElementId} className="dialog-title-text">
-                  {title}
-                </h2>
-                <div className="dialog-title-side">
-                  {!!onDone && (
-                    <button
-                      type="button"
-                      className="button button-borderless"
-                      onClick={onDone}
-                    >
-                      Done
-                    </button>
-                  )}
-                </div>
+        {!hideTitleBar &&
+          onDone && (
+            <div className="dialog-title-bar theme-color-border">
+              <div className="dialog-title-side" />
+              <h2 className="dialog-title-text">{title}</h2>
+              <div className="dialog-title-side">
+                {!!onDone && (
+                  <button
+                    type="button"
+                    aria-label="Close dialog"
+                    className="button button-borderless"
+                    onClick={onDone}
+                  >
+                    Done
+                  </button>
+                )}
               </div>
-            )}
-
-          <div ref={this.storeContent} className="dialog-content">
-            {children}
-          </div>
-        </div>
+            </div>
+          )}
 
-        <input
-          type="text"
-          className="focus-guard"
-          onFocus={this.focusFirstInputEvent}
-          spellCheck={false}
-        />
+        <div className="dialog-content">{children}</div>
       </div>
     );
   }