From e8b7782f5e4221024f182c3e675f6ebc82a5965c Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 13 Jan 2021 22:00:25 -0800 Subject: [PATCH] fix: Keyboard accessible context menus (#1768) - Makes menus fully accessible and keyboard driven - Currently adds 2.8% to initial bundle size due to the inclusion of Reakit and its dependency, popperjs. - Converts all menus to functional components - Remove old custom menu system - Various layout and flow improvements around the menus closes #1766 --- .flowconfig | 1 + app/components/Actions.js | 5 - app/components/Breadcrumb.js | 36 +- app/components/BreadcrumbMenu.js | 22 - app/components/Button.js | 29 +- app/components/ContextMenu/Header.js | 13 + .../MenuItem.js} | 38 +- .../ContextMenu/OverflowMenuButton.js | 21 + app/components/ContextMenu/Separator.js | 16 + .../Template.js} | 81 ++- app/components/ContextMenu/index.js | 77 +++ .../DocumentHistory/components/Revision.js | 5 +- app/components/DocumentListItem.js | 168 ++--- app/components/DocumentMeta.js | 1 + app/components/DropdownMenu/DropdownMenu.js | 289 -------- app/components/DropdownMenu/index.js | 3 - app/components/EventBoundary.js | 9 +- app/components/IconPicker.js | 157 ++--- app/components/Sidebar/Main.js | 9 +- .../Sidebar/components/CollectionLink.js | 2 - .../Sidebar/components/DocumentLink.js | 1 - .../Sidebar/components/HeaderBlock.js | 36 +- .../Sidebar/components/SidebarLink.js | 22 +- app/components/Toasts/components/Toast.js | 2 +- app/menus/AccountMenu.js | 218 +++--- app/menus/BreadcrumbMenu.js | 34 + app/menus/CollectionGroupMemberMenu.js | 44 ++ app/menus/CollectionMenu.js | 394 ++++++----- app/menus/CollectionSortMenu.js | 79 +-- app/menus/DocumentMenu.js | 627 +++++++++--------- app/menus/GroupMemberMenu.js | 36 + app/menus/GroupMenu.js | 156 ++--- app/menus/MemberMenu.js | 36 + app/menus/NewChildDocumentMenu.js | 75 +-- app/menus/NewDocumentMenu.js | 124 ++-- app/menus/NewTemplateMenu.js | 97 ++- app/menus/RevisionMenu.js | 99 +-- app/menus/ShareMenu.js | 104 ++- app/menus/TemplatesMenu.js | 62 +- app/menus/UserMenu.js | 168 ++--- app/scenes/Collection.js | 17 +- .../CollectionGroupMemberListItem.js | 15 +- .../components/MemberListItem.js | 10 +- app/scenes/Document/components/Header.js | 11 +- app/scenes/GroupMembers/AddPeopleToGroup.js | 1 - app/scenes/GroupMembers/GroupMembers.js | 1 - .../components/GroupMemberListItem.js | 21 +- app/scenes/Search/Search.js | 1 + app/scenes/Search/components/FilterOption.js | 48 +- app/scenes/Search/components/FilterOptions.js | 77 +-- app/utils/compressImage.js | 2 +- package.json | 1 + shared/i18n/locales/en_US/translation.json | 28 +- yarn.lock | 40 ++ 54 files changed, 1788 insertions(+), 1881 deletions(-) delete mode 100644 app/components/BreadcrumbMenu.js create mode 100644 app/components/ContextMenu/Header.js rename app/components/{DropdownMenu/DropdownMenuItem.js => ContextMenu/MenuItem.js} (68%) create mode 100644 app/components/ContextMenu/OverflowMenuButton.js create mode 100644 app/components/ContextMenu/Separator.js rename app/components/{DropdownMenu/DropdownMenuItems.js => ContextMenu/Template.js} (64%) create mode 100644 app/components/ContextMenu/index.js delete mode 100644 app/components/DropdownMenu/DropdownMenu.js delete mode 100644 app/components/DropdownMenu/index.js create mode 100644 app/menus/BreadcrumbMenu.js create mode 100644 app/menus/CollectionGroupMemberMenu.js create mode 100644 app/menus/GroupMemberMenu.js create mode 100644 app/menus/MemberMenu.js diff --git a/.flowconfig b/.flowconfig index 23473ecb0..37acb6c0a 100644 --- a/.flowconfig +++ b/.flowconfig @@ -32,6 +32,7 @@ module.file_ext=.json esproposal.decorators=ignore esproposal.class_static_fields=enable esproposal.class_instance_fields=enable +esproposal.optional_chaining=enable suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe suppress_comment=\\(.\\|\n\\)*\\$FlowIssue diff --git a/app/components/Actions.js b/app/components/Actions.js index 2e3fbf41d..77ebe565a 100644 --- a/app/components/Actions.js +++ b/app/components/Actions.js @@ -11,11 +11,6 @@ export const Action = styled(Flex)` font-size: 15px; flex-shrink: 0; - a { - color: ${(props) => props.theme.text}; - height: 24px; - } - &:empty { display: none; } diff --git a/app/components/Breadcrumb.js b/app/components/Breadcrumb.js index 16f5d9e04..b6130a16f 100644 --- a/app/components/Breadcrumb.js +++ b/app/components/Breadcrumb.js @@ -4,7 +4,6 @@ import { ArchiveIcon, EditIcon, GoToIcon, - MoreIcon, PadlockIcon, ShapesIcon, TrashIcon, @@ -14,18 +13,15 @@ import { useTranslation } from "react-i18next"; import { Link } from "react-router-dom"; import styled from "styled-components"; import breakpoint from "styled-components-breakpoint"; - -import CollectionsStore from "stores/CollectionsStore"; import Document from "models/Document"; import CollectionIcon from "components/CollectionIcon"; import Flex from "components/Flex"; -import BreadcrumbMenu from "./BreadcrumbMenu"; import useStores from "hooks/useStores"; +import BreadcrumbMenu from "menus/BreadcrumbMenu"; import { collectionUrl } from "utils/routeHelpers"; type Props = { document: Document, - collections: CollectionsStore, onlyText: boolean, }; @@ -133,7 +129,7 @@ const Breadcrumb = ({ document, onlyText }: Props) => { {isNestedDocument && ( <> - } path={menuPath} /> + )} {lastPath && ( @@ -148,6 +144,11 @@ const Breadcrumb = ({ document, onlyText }: Props) => { ); }; +export const Slash = styled(GoToIcon)` + flex-shrink: 0; + fill: ${(props) => props.theme.divider}; +`; + const Wrapper = styled(Flex)` display: none; @@ -168,22 +169,6 @@ const SmallSlash = styled(GoToIcon)` opacity: 0.25; `; -export const Slash = styled(GoToIcon)` - flex-shrink: 0; - fill: ${(props) => props.theme.divider}; -`; - -const Overflow = styled(MoreIcon)` - flex-shrink: 0; - transition: opacity 100ms ease-in-out; - fill: ${(props) => props.theme.divider}; - - &:active, - &:hover { - fill: ${(props) => props.theme.text}; - } -`; - const Crumb = styled(Link)` color: ${(props) => props.theme.text}; font-size: 15px; @@ -199,12 +184,17 @@ const Crumb = styled(Link)` const CollectionName = styled(Link)` display: flex; - flex-shrink: 0; + flex-shrink: 1; color: ${(props) => props.theme.text}; font-size: 15px; font-weight: 500; white-space: nowrap; overflow: hidden; + min-width: 0; + + svg { + flex-shrink: 0; + } `; export default observer(Breadcrumb); diff --git a/app/components/BreadcrumbMenu.js b/app/components/BreadcrumbMenu.js deleted file mode 100644 index fa7df3f4b..000000000 --- a/app/components/BreadcrumbMenu.js +++ /dev/null @@ -1,22 +0,0 @@ -// @flow -import * as React from "react"; -import { DropdownMenu } from "components/DropdownMenu"; -import DropdownMenuItems from "components/DropdownMenu/DropdownMenuItems"; - -type Props = { - label: React.Node, - path: Array, -}; - -export default function BreadcrumbMenu({ label, path }: Props) { - return ( - - ({ - title: item.title, - to: item.url, - }))} - /> - - ); -} diff --git a/app/components/Button.js b/app/components/Button.js index d978887b5..14b878a90 100644 --- a/app/components/Button.js +++ b/app/components/Button.js @@ -22,9 +22,13 @@ const RealButton = styled.button` cursor: pointer; user-select: none; - svg { - fill: ${(props) => props.iconColor || props.theme.buttonText}; - } + ${(props) => + !props.borderOnHover && + ` + svg { + fill: ${props.iconColor || props.theme.buttonText}; + } + `} &::-moz-focus-inner { padding: 0; @@ -42,7 +46,7 @@ const RealButton = styled.button` } ${(props) => - props.neutral && + props.$neutral && ` background: ${props.theme.buttonNeutralBackground}; color: ${props.theme.buttonNeutralText}; @@ -52,9 +56,14 @@ const RealButton = styled.button` : `rgba(0, 0, 0, 0.07) 0px 1px 2px, ${props.theme.buttonNeutralBorder} 0 0 0 1px inset` }; - svg { + ${ + props.borderOnHover + ? "" + : `svg { fill: ${props.iconColor || props.theme.buttonNeutralText}; + }` } + &:hover { background: ${darken(0.05, props.theme.buttonNeutralBackground)}; @@ -72,9 +81,9 @@ const RealButton = styled.button` background: ${props.theme.danger}; color: ${props.theme.white}; - &:hover { - background: ${darken(0.05, props.theme.danger)}; - } + &:hover { + background: ${darken(0.05, props.theme.danger)}; + } `}; `; @@ -108,6 +117,7 @@ export type Props = { children?: React.Node, innerRef?: React.ElementRef, disclosure?: boolean, + neutral?: boolean, fullwidth?: boolean, borderOnHover?: boolean, }; @@ -119,13 +129,14 @@ function Button({ value, disclosure, innerRef, + neutral, ...rest }: Props) { const hasText = children !== undefined || value !== undefined; const hasIcon = icon !== undefined; return ( - + {hasIcon && icon} {hasText && } diff --git a/app/components/ContextMenu/Header.js b/app/components/ContextMenu/Header.js new file mode 100644 index 000000000..775991eb8 --- /dev/null +++ b/app/components/ContextMenu/Header.js @@ -0,0 +1,13 @@ +// @flow +import styled from "styled-components"; + +const Header = styled.h3` + font-size: 11px; + font-weight: 600; + text-transform: uppercase; + color: ${(props) => props.theme.sidebarText}; + letter-spacing: 0.04em; + margin: 1em 12px 0.5em; +`; + +export default Header; diff --git a/app/components/DropdownMenu/DropdownMenuItem.js b/app/components/ContextMenu/MenuItem.js similarity index 68% rename from app/components/DropdownMenu/DropdownMenuItem.js rename to app/components/ContextMenu/MenuItem.js index 95269e1d2..089144ce9 100644 --- a/app/components/DropdownMenu/DropdownMenuItem.js +++ b/app/components/ContextMenu/MenuItem.js @@ -1,6 +1,7 @@ // @flow import { CheckmarkIcon } from "outline-icons"; import * as React from "react"; +import { MenuItem as BaseMenuItem } from "reakit/Menu"; import styled from "styled-components"; type Props = { @@ -8,31 +9,35 @@ type Props = { children?: React.Node, selected?: boolean, disabled?: boolean, + as?: string | React.ComponentType<*>, }; -const DropdownMenuItem = ({ +const MenuItem = ({ onClick, children, selected, disabled, + as, ...rest }: Props) => { return ( - - {selected !== undefined && ( - <> - {selected ? : } -   - + {(props) => ( + + {selected !== undefined && ( + <> + {selected ? : } +   + + )} + {children} + )} - {children} - + ); }; @@ -41,13 +46,14 @@ const Spacer = styled.div` height: 24px; `; -const MenuItem = styled.a` +export const MenuAnchor = styled.a` display: flex; margin: 0; + border: 0; padding: 6px 12px; width: 100%; min-height: 32px; - + background: none; color: ${(props) => props.disabled ? props.theme.textTertiary : props.theme.textSecondary}; justify-content: left; @@ -61,6 +67,7 @@ const MenuItem = styled.a` } svg { + flex-shrink: 0; opacity: ${(props) => (props.disabled ? ".5" : 1)}; } @@ -69,7 +76,8 @@ const MenuItem = styled.a` ? "pointer-events: none;" : ` - &:hover { + &:hover, + &.focus-visible { color: ${props.theme.white}; background: ${props.theme.primary}; box-shadow: none; @@ -87,4 +95,4 @@ const MenuItem = styled.a` `}; `; -export default DropdownMenuItem; +export default MenuItem; diff --git a/app/components/ContextMenu/OverflowMenuButton.js b/app/components/ContextMenu/OverflowMenuButton.js new file mode 100644 index 000000000..419b63b59 --- /dev/null +++ b/app/components/ContextMenu/OverflowMenuButton.js @@ -0,0 +1,21 @@ +// @flow +import { MoreIcon } from "outline-icons"; +import * as React from "react"; +import { MenuButton } from "reakit/Menu"; +import NudeButton from "components/NudeButton"; + +export default function OverflowMenuButton({ + iconColor, + className, + ...rest +}: any) { + return ( + + {(props) => ( + + + + )} + + ); +} diff --git a/app/components/ContextMenu/Separator.js b/app/components/ContextMenu/Separator.js new file mode 100644 index 000000000..5dbc29c5c --- /dev/null +++ b/app/components/ContextMenu/Separator.js @@ -0,0 +1,16 @@ +// @flow +import * as React from "react"; +import { MenuSeparator } from "reakit/Menu"; +import styled from "styled-components"; + +export default function Separator(rest: {}) { + return ( + + {(props) => } + + ); +} + +const HorizontalRule = styled.hr` + margin: 0.5em 12px; +`; diff --git a/app/components/DropdownMenu/DropdownMenuItems.js b/app/components/ContextMenu/Template.js similarity index 64% rename from app/components/DropdownMenu/DropdownMenuItems.js rename to app/components/ContextMenu/Template.js index 3752c4ec1..d1ab6b700 100644 --- a/app/components/DropdownMenu/DropdownMenuItems.js +++ b/app/components/ContextMenu/Template.js @@ -1,13 +1,19 @@ // @flow import { ExpandedIcon } from "outline-icons"; import * as React from "react"; +import { useTranslation } from "react-i18next"; import { Link } from "react-router-dom"; +import { + useMenuState, + MenuButton, + MenuItem as BaseMenuItem, +} from "reakit/Menu"; import styled from "styled-components"; -import Flex from "components/Flex"; -import DropdownMenu from "./DropdownMenu"; -import DropdownMenuItem from "./DropdownMenuItem"; +import MenuItem, { MenuAnchor } from "./MenuItem"; +import Separator from "./Separator"; +import ContextMenu from "."; -type MenuItem = +type TMenuItem = | {| title: React.Node, to: string, @@ -35,7 +41,7 @@ type MenuItem = disabled?: boolean, style?: Object, hover?: boolean, - items: MenuItem[], + items: TMenuItem[], |} | {| type: "separator", @@ -48,14 +54,35 @@ type MenuItem = |}; type Props = {| - items: MenuItem[], + items: TMenuItem[], |}; const Disclosure = styled(ExpandedIcon)` transform: rotate(270deg); + justify-self: flex-end; `; -export default function DropdownMenuItems({ items }: Props): React.Node { +const Submenu = React.forwardRef(({ templateItems, title, ...rest }, ref) => { + const { t } = useTranslation(); + const menu = useMenuState({ modal: true }); + + return ( + <> + + {(props) => ( + + {title} + + )} + + +