From 6520a501e386c3a9ba684129cdd18dcca5ea2fec Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Fri, 30 Aug 2019 00:27:40 -0700
Subject: [PATCH] fix: accessiblity improvements, focus states, real buttons

---
 app/components/Button.js                      | 13 ++++++++++
 app/components/Editor/Editor.js               |  6 +++--
 app/components/NudeButton.js                  | 26 +++++++++++++++++++
 app/components/Tab.js                         |  8 ++++++
 app/components/Tooltip.js                     |  4 ---
 app/menus/CollectionMenu.js                   | 10 ++++---
 app/menus/DocumentMenu.js                     | 18 ++++++++++---
 app/menus/RevisionMenu.js                     | 10 ++++---
 app/menus/ShareMenu.js                        | 10 ++++---
 app/menus/UserMenu.js                         |  9 ++++++-
 .../components/MemberListItem.js              |  9 ++++++-
 app/scenes/Document/components/Header.js      | 24 ++++++++++-------
 .../Document/components/KeyboardShortcuts.js  | 23 ++--------------
 app/scenes/Invite.js                          |  5 +++-
 14 files changed, 123 insertions(+), 52 deletions(-)
 create mode 100644 app/components/NudeButton.js

diff --git a/app/components/Button.js b/app/components/Button.js
index 94a541a95..a7daa2ee5 100644
--- a/app/components/Button.js
+++ b/app/components/Button.js
@@ -65,6 +65,13 @@ const RealButton = styled.button`
       border: 1px solid ${darken(0.15, props.theme.buttonNeutralBackground)};
     }
 
+    &:focus {
+      transition-duration: 0.05s;
+      border: 1px solid ${lighten(0.4, props.theme.buttonBackground)};
+      box-shadow: ${lighten(0.4, props.theme.buttonBackground)} 0px 0px
+        0px 2px;
+    }
+
     &:disabled {
       color: ${props.theme.textTertiary};
     }
@@ -77,6 +84,12 @@ const RealButton = styled.button`
     &:hover {
       background: ${darken(0.05, props.theme.danger)};
     }
+
+    &:focus {
+      transition-duration: 0.05s;
+      box-shadow: ${lighten(0.4, props.theme.danger)} 0px 0px
+        0px 3px;
+    }
   `};
 `;
 
diff --git a/app/components/Editor/Editor.js b/app/components/Editor/Editor.js
index 48c474a9a..4fa087693 100644
--- a/app/components/Editor/Editor.js
+++ b/app/components/Editor/Editor.js
@@ -272,8 +272,10 @@ const PrismStyles = createGlobalStyle`
   }
 `;
 
-const EditorTooltip = props => (
-  <Tooltip offset="0, 16" delay={150} {...props} />
+const EditorTooltip = ({ children, ...props }) => (
+  <Tooltip offset="0, 16" delay={150} {...props}>
+    <span>{children}</span>
+  </Tooltip>
 );
 
 export default withTheme(
diff --git a/app/components/NudeButton.js b/app/components/NudeButton.js
new file mode 100644
index 000000000..7a847f9df
--- /dev/null
+++ b/app/components/NudeButton.js
@@ -0,0 +1,26 @@
+// @flow
+import * as React from 'react';
+import styled from 'styled-components';
+import { lighten } from 'polished';
+
+const Button = styled.button`
+  width: 24px;
+  height: 24px;
+  background: none;
+  border-radius: 4px;
+  line-height: 0;
+  border: 0;
+  padding: 0;
+
+  &:focus {
+    transition-duration: 0.05s;
+    box-shadow: ${props => lighten(0.4, props.theme.buttonBackground)} 0px 0px
+      0px 3px;
+    outline: none;
+  }
+`;
+
+// $FlowFixMe - need to upgrade to get forwardRef
+export default React.forwardRef((props, ref) => (
+  <Button {...props} ref={ref} />
+));
diff --git a/app/components/Tab.js b/app/components/Tab.js
index 824b89efe..48b1ac8e9 100644
--- a/app/components/Tab.js
+++ b/app/components/Tab.js
@@ -2,6 +2,7 @@
 import * as React from 'react';
 import styled, { withTheme } from 'styled-components';
 import { NavLink } from 'react-router-dom';
+import { lighten } from 'polished';
 
 type Props = {
   theme: Object,
@@ -23,6 +24,13 @@ const StyledNavLink = styled(NavLink)`
     border-bottom: 3px solid ${props => props.theme.divider};
     padding-bottom: 5px;
   }
+
+  &:focus {
+    outline: none;
+    border-bottom: 3px solid
+      ${props => lighten(0.4, props.theme.buttonBackground)};
+    padding-bottom: 5px;
+  }
 `;
 
 function Tab(props: Props) {
diff --git a/app/components/Tooltip.js b/app/components/Tooltip.js
index 06c34da8b..ddc1b56c9 100644
--- a/app/components/Tooltip.js
+++ b/app/components/Tooltip.js
@@ -13,10 +13,6 @@ type Props = {
 };
 
 class Tooltip extends React.Component<Props> {
-  shouldComponentUpdate() {
-    return false;
-  }
-
   render() {
     const { shortcut, tooltip, delay = 50, className, ...rest } = this.props;
 
diff --git a/app/menus/CollectionMenu.js b/app/menus/CollectionMenu.js
index 23c3d34aa..941c3c01b 100644
--- a/app/menus/CollectionMenu.js
+++ b/app/menus/CollectionMenu.js
@@ -15,9 +15,9 @@ import Collection from 'models/Collection';
 import UiStore from 'stores/UiStore';
 import DocumentsStore from 'stores/DocumentsStore';
 import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu';
+import NudeButton from 'components/NudeButton';
 
 type Props = {
-  label?: React.Node,
   position?: 'left' | 'right' | 'center',
   ui: UiStore,
   documents: DocumentsStore,
@@ -89,7 +89,7 @@ class CollectionMenu extends React.Component<Props> {
   };
 
   render() {
-    const { collection, label, position, onOpen, onClose } = this.props;
+    const { collection, position, onOpen, onClose } = this.props;
 
     return (
       <React.Fragment>
@@ -110,7 +110,11 @@ class CollectionMenu extends React.Component<Props> {
           />
         </Modal>
         <DropdownMenu
-          label={label || <MoreIcon />}
+          label={
+            <NudeButton>
+              <MoreIcon />
+            </NudeButton>
+          }
           onOpen={onOpen}
           onClose={onClose}
           position={position}
diff --git a/app/menus/DocumentMenu.js b/app/menus/DocumentMenu.js
index e2f7a6092..1e3c4484b 100644
--- a/app/menus/DocumentMenu.js
+++ b/app/menus/DocumentMenu.js
@@ -16,11 +16,11 @@ import {
   newDocumentUrl,
 } from 'utils/routeHelpers';
 import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu';
+import NudeButton from 'components/NudeButton';
 
 type Props = {
   ui: UiStore,
   auth: AuthStore,
-  label?: React.Node,
   position?: 'left' | 'right' | 'center',
   document: Document,
   collections: CollectionStore,
@@ -113,7 +113,6 @@ class DocumentMenu extends React.Component<Props> {
     const {
       document,
       position,
-      label,
       className,
       showPrint,
       showPin,
@@ -125,7 +124,14 @@ class DocumentMenu extends React.Component<Props> {
 
     if (document.isArchived) {
       return (
-        <DropdownMenu label={label || <MoreIcon />} className={className}>
+        <DropdownMenu
+          label={
+            <NudeButton>
+              <MoreIcon />
+            </NudeButton>
+          }
+          className={className}
+        >
           <DropdownMenuItem onClick={this.handleRestore}>
             Restore
           </DropdownMenuItem>
@@ -138,7 +144,11 @@ class DocumentMenu extends React.Component<Props> {
 
     return (
       <DropdownMenu
-        label={label || <MoreIcon />}
+        label={
+          <NudeButton>
+            <MoreIcon />
+          </NudeButton>
+        }
         className={className}
         position={position}
         onOpen={onOpen}
diff --git a/app/menus/RevisionMenu.js b/app/menus/RevisionMenu.js
index 7a7ebf95f..5a012b72b 100644
--- a/app/menus/RevisionMenu.js
+++ b/app/menus/RevisionMenu.js
@@ -4,6 +4,7 @@ import { withRouter, type RouterHistory } from 'react-router-dom';
 import { inject } from 'mobx-react';
 import { MoreIcon } from 'outline-icons';
 
+import NudeButton from 'components/NudeButton';
 import CopyToClipboard from 'components/CopyToClipboard';
 import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu';
 import { documentHistoryUrl } from 'utils/routeHelpers';
@@ -12,7 +13,6 @@ import Document from 'models/Document';
 import UiStore from 'stores/UiStore';
 
 type Props = {
-  label?: React.Node,
   onOpen?: () => void,
   onClose: () => void,
   history: RouterHistory,
@@ -35,7 +35,7 @@ class RevisionMenu extends React.Component<Props> {
   };
 
   render() {
-    const { label, className, onOpen, onClose } = this.props;
+    const { className, onOpen, onClose } = this.props;
     const url = `${window.location.origin}${documentHistoryUrl(
       this.props.document,
       this.props.revision.id
@@ -43,7 +43,11 @@ class RevisionMenu extends React.Component<Props> {
 
     return (
       <DropdownMenu
-        label={label || <MoreIcon />}
+        label={
+          <NudeButton>
+            <MoreIcon />
+          </NudeButton>
+        }
         onOpen={onOpen}
         onClose={onClose}
         className={className}
diff --git a/app/menus/ShareMenu.js b/app/menus/ShareMenu.js
index a86a05915..808cf1a6f 100644
--- a/app/menus/ShareMenu.js
+++ b/app/menus/ShareMenu.js
@@ -5,6 +5,7 @@ import { inject, observer } from 'mobx-react';
 import { observable } from 'mobx';
 import { MoreIcon } from 'outline-icons';
 
+import NudeButton from 'components/NudeButton';
 import CopyToClipboard from 'components/CopyToClipboard';
 import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu';
 import SharesStore from 'stores/SharesStore';
@@ -12,7 +13,6 @@ import UiStore from 'stores/UiStore';
 import Share from 'models/Share';
 
 type Props = {
-  label?: React.Node,
   onOpen?: () => void,
   onClose: () => void,
   shares: SharesStore,
@@ -46,11 +46,15 @@ class ShareMenu extends React.Component<Props> {
   render() {
     if (this.redirectTo) return <Redirect to={this.redirectTo} push />;
 
-    const { share, label, onOpen, onClose } = this.props;
+    const { share, onOpen, onClose } = this.props;
 
     return (
       <DropdownMenu
-        label={label || <MoreIcon />}
+        label={
+          <NudeButton>
+            <MoreIcon />
+          </NudeButton>
+        }
         onOpen={onOpen}
         onClose={onClose}
       >
diff --git a/app/menus/UserMenu.js b/app/menus/UserMenu.js
index eda16d862..83005bb33 100644
--- a/app/menus/UserMenu.js
+++ b/app/menus/UserMenu.js
@@ -4,6 +4,7 @@ import { inject, observer } from 'mobx-react';
 import { MoreIcon } from 'outline-icons';
 
 import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu';
+import NudeButton from 'components/NudeButton';
 import UsersStore from 'stores/UsersStore';
 import User from 'models/User';
 
@@ -61,7 +62,13 @@ class UserMenu extends React.Component<Props> {
     const { user } = this.props;
 
     return (
-      <DropdownMenu label={<MoreIcon />}>
+      <DropdownMenu
+        label={
+          <NudeButton>
+            <MoreIcon />
+          </NudeButton>
+        }
+      >
         {!user.isSuspended &&
           (user.isAdmin ? (
             <DropdownMenuItem onClick={this.handleDemote}>
diff --git a/app/scenes/CollectionPermissions/components/MemberListItem.js b/app/scenes/CollectionPermissions/components/MemberListItem.js
index eed20acff..98730c00a 100644
--- a/app/scenes/CollectionPermissions/components/MemberListItem.js
+++ b/app/scenes/CollectionPermissions/components/MemberListItem.js
@@ -8,6 +8,7 @@ import Flex from 'shared/components/Flex';
 import ListItem from 'components/List/Item';
 import User from 'models/User';
 import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu';
+import NudeButton from 'components/NudeButton';
 
 type Props = {
   user: User,
@@ -24,7 +25,13 @@ const MemberListItem = ({ user, onRemove, showRemove }: Props) => {
         <Flex align="center">
           <Permission as="span">Can edit&nbsp;</Permission>
           {showRemove && (
-            <DropdownMenu label={<MoreIcon />}>
+            <DropdownMenu
+              label={
+                <NudeButton>
+                  <MoreIcon />
+                </NudeButton>
+              }
+            >
               <DropdownMenuItem onClick={onRemove}>Remove</DropdownMenuItem>
             </DropdownMenu>
           )}
diff --git a/app/scenes/Document/components/Header.js b/app/scenes/Document/components/Header.js
index 2ac3fed12..744f3eecb 100644
--- a/app/scenes/Document/components/Header.js
+++ b/app/scenes/Document/components/Header.js
@@ -156,16 +156,22 @@ class Header extends React.Component<Props> {
           {isEditing && (
             <React.Fragment>
               <Action>
-                <Button
-                  onClick={this.handleSave}
-                  title={`Save changes (${meta}+Enter)`}
-                  disabled={savingIsDisabled}
-                  isSaving={isSaving}
-                  neutral={isDraft}
-                  small
+                <Tooltip
+                  tooltip="Save"
+                  shortcut={`${meta}+enter`}
+                  delay={500}
+                  placement="bottom"
                 >
-                  {isDraft ? 'Save Draft' : 'Done Editing'}
-                </Button>
+                  <Button
+                    onClick={this.handleSave}
+                    disabled={savingIsDisabled}
+                    isSaving={isSaving}
+                    neutral={isDraft}
+                    small
+                  >
+                    {isDraft ? 'Save Draft' : 'Done Editing'}
+                  </Button>
+                </Tooltip>
               </Action>
             </React.Fragment>
           )}
diff --git a/app/scenes/Document/components/KeyboardShortcuts.js b/app/scenes/Document/components/KeyboardShortcuts.js
index 5d82873b2..a8dceea3e 100644
--- a/app/scenes/Document/components/KeyboardShortcuts.js
+++ b/app/scenes/Document/components/KeyboardShortcuts.js
@@ -2,12 +2,12 @@
 import * as React from 'react';
 import styled from 'styled-components';
 import breakpoint from 'styled-components-breakpoint';
-import { lighten } from 'polished';
 import { observable } from 'mobx';
 import { observer } from 'mobx-react';
 import { KeyboardIcon } from 'outline-icons';
 import Modal from 'components/Modal';
 import Tooltip from 'components/Tooltip';
+import NudeButton from 'components/NudeButton';
 import KeyboardShortcuts from 'scenes/KeyboardShortcuts';
 
 type Props = {};
@@ -49,31 +49,12 @@ class KeyboardShortcutsButton extends React.Component<Props> {
   }
 }
 
-const Button = styled.button`
+const Button = styled(NudeButton)`
   display: none;
   position: fixed;
   bottom: 0;
   right: 0;
   margin: 24px;
-  width: 24px;
-  height: 24px;
-  opacity: 0.8;
-  background: none;
-  border-radius: 4px;
-  line-height: 0;
-  border: 0;
-  padding: 0;
-
-  &:hover {
-    opacity: 1;
-  }
-
-  &:focus {
-    transition-duration: 0.05s;
-    box-shadow: ${props => lighten(0.4, props.theme.buttonBackground)} 0px 0px
-      0px 3px;
-    outline: none;
-  }
 
   ${breakpoint('tablet')`
     display: block;
diff --git a/app/scenes/Invite.js b/app/scenes/Invite.js
index b04a578f2..22a13d077 100644
--- a/app/scenes/Invite.js
+++ b/app/scenes/Invite.js
@@ -11,6 +11,7 @@ import Button from 'components/Button';
 import Input from 'components/Input';
 import HelpText from 'components/HelpText';
 import Tooltip from 'components/Tooltip';
+import NudeButton from 'components/NudeButton';
 
 import UiStore from 'stores/UiStore';
 import AuthStore from 'stores/AuthStore';
@@ -124,7 +125,9 @@ class Invite extends React.Component<Props> {
             {index !== 0 && (
               <Remove>
                 <Tooltip tooltip="Remove invite" placement="top">
-                  <CloseIcon onClick={() => this.handleRemove(index)} />
+                  <NudeButton onClick={() => this.handleRemove(index)}>
+                    <CloseIcon />
+                  </NudeButton>
                 </Tooltip>
               </Remove>
             )}