From 23c8adc5d1a79375e93e29735b2d961a0d5991d5 Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Thu, 13 Jun 2024 18:45:44 +0530 Subject: [PATCH] Replace `reakit/Composite` with `react-roving-tabindex` (#6985) * fix: replace reakit composite with react-roving-tabindex * fix: touch points * fix: focus stuck at first list item * fix: document history navigation * fix: remove ununsed ListItem components * fix: keyboard navigation in recent search list * fix: updated lib --- app/components/ArrowKeyNavigation.tsx | 32 ++---- app/components/DocumentListItem.tsx | 24 +++- app/components/EventListItem.tsx | 18 +-- app/components/List/CompositeItem.tsx | 17 --- app/components/List/Item.tsx | 27 ++++- app/components/PaginatedDocumentList.tsx | 3 +- app/components/PaginatedEventList.tsx | 3 +- app/components/PaginatedList.tsx | 11 +- app/components/SearchListItem.tsx | 28 ++++- app/components/SearchPopover.tsx | 3 +- .../GroupMembers/components/UserListItem.tsx | 49 --------- app/scenes/Search/Search.tsx | 28 ++--- .../components/RecentSearchListItem.tsx | 92 ++++++++++++++++ .../Search/components/RecentSearches.tsx | 103 +++--------------- .../Settings/components/ShareListItem.tsx | 39 ------- .../Settings/components/UserListItem.tsx | 50 --------- package.json | 1 + shared/i18n/locales/en_US/translation.json | 6 +- yarn.lock | 14 +++ 19 files changed, 219 insertions(+), 329 deletions(-) delete mode 100644 app/components/List/CompositeItem.tsx delete mode 100644 app/scenes/GroupMembers/components/UserListItem.tsx create mode 100644 app/scenes/Search/components/RecentSearchListItem.tsx delete mode 100644 app/scenes/Settings/components/ShareListItem.tsx delete mode 100644 app/scenes/Settings/components/UserListItem.tsx diff --git a/app/components/ArrowKeyNavigation.tsx b/app/components/ArrowKeyNavigation.tsx index f0f3e3d5f..1c714f1fa 100644 --- a/app/components/ArrowKeyNavigation.tsx +++ b/app/components/ArrowKeyNavigation.tsx @@ -1,13 +1,9 @@ +import { RovingTabIndexProvider } from "@getoutline/react-roving-tabindex"; import { observer } from "mobx-react"; import * as React from "react"; -import { - useCompositeState, - Composite, - CompositeStateReturn, -} from "reakit/Composite"; type Props = React.HTMLAttributes & { - children: (composite: CompositeStateReturn) => React.ReactNode; + children: () => React.ReactNode; onEscape?: (ev: React.KeyboardEvent) => void; }; @@ -15,40 +11,36 @@ function ArrowKeyNavigation( { children, onEscape, ...rest }: Props, ref: React.RefObject ) { - const composite = useCompositeState(); - const handleKeyDown = React.useCallback( - (ev) => { + (ev: React.KeyboardEvent) => { if (onEscape) { if (ev.nativeEvent.isComposing) { return; } if (ev.key === "Escape") { + ev.preventDefault(); onEscape(ev); } if ( ev.key === "ArrowUp" && - composite.currentId === composite.items[0].id + // If the first item is focused and the user presses ArrowUp + ev.currentTarget.firstElementChild === document.activeElement ) { onEscape(ev); } } }, - [composite.currentId, composite.items, onEscape] + [onEscape] ); return ( - - {children(composite)} - + +
+ {children()} +
+ ); } diff --git a/app/components/DocumentListItem.tsx b/app/components/DocumentListItem.tsx index 999f28cb5..d431c3e53 100644 --- a/app/components/DocumentListItem.tsx +++ b/app/components/DocumentListItem.tsx @@ -1,8 +1,11 @@ +import { + useFocusEffect, + useRovingTabIndex, +} from "@getoutline/react-roving-tabindex"; import { observer } from "mobx-react"; import * as React from "react"; import { useTranslation } from "react-i18next"; import { Link } from "react-router-dom"; -import { CompositeStateReturn, CompositeItem } from "reakit/Composite"; import styled, { css } from "styled-components"; import breakpoint from "styled-components-breakpoint"; import { s } from "@shared/styles"; @@ -32,7 +35,7 @@ type Props = { showPin?: boolean; showDraft?: boolean; showTemplate?: boolean; -} & CompositeStateReturn; +}; const SEARCH_RESULT_REGEX = /]*>(.*?)<\/b>/gi; @@ -49,6 +52,15 @@ function DocumentListItem( const user = useCurrentUser(); const [menuOpen, handleMenuOpen, handleMenuClose] = useBoolean(); + let itemRef: React.Ref = + React.useRef(null); + if (ref) { + itemRef = ref; + } + + const { focused, ...rovingTabIndex } = useRovingTabIndex(itemRef, false); + useFocusEffect(focused, itemRef); + const { document, showParentDocuments, @@ -68,9 +80,8 @@ function DocumentListItem( !document.isDraft && !document.isArchived && !document.isTemplate; return ( - @@ -142,7 +154,7 @@ function DocumentListItem( modal={false} /> - + ); } diff --git a/app/components/EventListItem.tsx b/app/components/EventListItem.tsx index abe87e8b9..e9da92943 100644 --- a/app/components/EventListItem.tsx +++ b/app/components/EventListItem.tsx @@ -11,16 +11,12 @@ import { import * as React from "react"; import { useTranslation } from "react-i18next"; import { useLocation } from "react-router-dom"; -import { CompositeStateReturn } from "reakit/Composite"; import styled, { css } from "styled-components"; import { s } from "@shared/styles"; import Document from "~/models/Document"; import Event from "~/models/Event"; import Avatar from "~/components/Avatar"; -import CompositeItem, { - Props as ItemProps, -} from "~/components/List/CompositeItem"; -import Item, { Actions } from "~/components/List/Item"; +import Item, { Actions, Props as ItemProps } from "~/components/List/Item"; import Time from "~/components/Time"; import useStores from "~/hooks/useStores"; import RevisionMenu from "~/menus/RevisionMenu"; @@ -32,7 +28,7 @@ type Props = { document: Document; event: Event; latest?: boolean; -} & CompositeStateReturn; +}; const EventListItem = ({ event, latest, document, ...rest }: Props) => { const { t } = useTranslation(); @@ -176,11 +172,7 @@ const BaseItem = React.forwardRef(function _BaseItem( { to, ...rest }: ItemProps, ref?: React.Ref ) { - if (to) { - return ; - } - - return ; + return ; }); const Subtitle = styled.span` @@ -240,8 +232,4 @@ const ListItem = styled(Item)` ${ItemStyle} `; -const CompositeListItem = styled(CompositeItem)` - ${ItemStyle} -`; - export default observer(EventListItem); diff --git a/app/components/List/CompositeItem.tsx b/app/components/List/CompositeItem.tsx deleted file mode 100644 index 0d9d9583f..000000000 --- a/app/components/List/CompositeItem.tsx +++ /dev/null @@ -1,17 +0,0 @@ -import * as React from "react"; -import { - CompositeStateReturn, - CompositeItem as BaseCompositeItem, -} from "reakit/Composite"; -import Item, { Props as ItemProps } from "./Item"; - -export type Props = ItemProps & CompositeStateReturn; - -function CompositeItem( - { to, ...rest }: Props, - ref?: React.Ref -) { - return ; -} - -export default React.forwardRef(CompositeItem); diff --git a/app/components/List/Item.tsx b/app/components/List/Item.tsx index d43240c6b..df267991c 100644 --- a/app/components/List/Item.tsx +++ b/app/components/List/Item.tsx @@ -1,3 +1,7 @@ +import { + useFocusEffect, + useRovingTabIndex, +} from "@getoutline/react-roving-tabindex"; import { LocationDescriptor } from "history"; import * as React from "react"; import styled, { useTheme } from "styled-components"; @@ -33,6 +37,18 @@ const ListItem = ( const theme = useTheme(); const compact = !subtitle; + let itemRef: React.Ref = + React.useRef(null); + if (ref) { + itemRef = ref; + } + + const { focused, ...rovingTabIndex } = useRovingTabIndex( + itemRef as React.RefObject, + to ? false : true + ); + useFocusEffect(focused, itemRef as React.RefObject); + const content = (selected: boolean) => ( <> {image && {image}} @@ -59,13 +75,20 @@ const ListItem = ( if (to) { return ( { + if (rest.onClick) { + rest.onClick(ev); + } + rovingTabIndex.onClick(ev); + }} as={NavLink} to={to} > @@ -75,7 +98,7 @@ const ListItem = ( } return ( - + {content(false)} ); diff --git a/app/components/PaginatedDocumentList.tsx b/app/components/PaginatedDocumentList.tsx index 1b93c5bb1..3e4073982 100644 --- a/app/components/PaginatedDocumentList.tsx +++ b/app/components/PaginatedDocumentList.tsx @@ -42,7 +42,7 @@ const PaginatedDocumentList = React.memo(function PaginatedDocumentList({ fetch={fetch} options={options} renderError={(props) => } - renderItem={(item: Document, _index, compositeProps) => ( + renderItem={(item: Document, _index) => ( (function PaginatedDocumentList({ showPublished={showPublished} showTemplate={showTemplate} showDraft={showDraft} - {...compositeProps} /> )} {...rest} diff --git a/app/components/PaginatedEventList.tsx b/app/components/PaginatedEventList.tsx index dfc46adbf..6de167b0e 100644 --- a/app/components/PaginatedEventList.tsx +++ b/app/components/PaginatedEventList.tsx @@ -30,13 +30,12 @@ const PaginatedEventList = React.memo(function PaginatedEventList({ heading={heading} fetch={fetch} options={options} - renderItem={(item: Event, index, compositeProps) => ( + renderItem={(item: Event, index) => ( )} renderHeading={(name) => {name}} diff --git a/app/components/PaginatedList.tsx b/app/components/PaginatedList.tsx index 8f6b86662..87661d084 100644 --- a/app/components/PaginatedList.tsx +++ b/app/components/PaginatedList.tsx @@ -4,7 +4,6 @@ import { observer } from "mobx-react"; import * as React from "react"; import { withTranslation, WithTranslation } from "react-i18next"; import { Waypoint } from "react-waypoint"; -import { CompositeStateReturn } from "reakit/Composite"; import { Pagination } from "@shared/constants"; import RootStore from "~/stores/RootStore"; import ArrowKeyNavigation from "~/components/ArrowKeyNavigation"; @@ -30,11 +29,7 @@ type Props = WithTranslation & loading?: React.ReactElement; items?: T[]; className?: string; - renderItem: ( - item: T, - index: number, - compositeProps: CompositeStateReturn - ) => React.ReactNode; + renderItem: (item: T, index: number) => React.ReactNode; renderError?: (options: { error: Error; retry: () => void; @@ -194,10 +189,10 @@ class PaginatedList extends React.Component> { onEscape={onEscape} className={this.props.className} > - {(composite: CompositeStateReturn) => { + {() => { let previousHeading = ""; return items.slice(0, this.renderCount).map((item, index) => { - const children = this.props.renderItem(item, index, composite); + const children = this.props.renderItem(item, index); // If there is no renderHeading method passed then no date // headings are rendered diff --git a/app/components/SearchListItem.tsx b/app/components/SearchListItem.tsx index e9c5a98eb..547a77c9d 100644 --- a/app/components/SearchListItem.tsx +++ b/app/components/SearchListItem.tsx @@ -1,7 +1,10 @@ +import { + useFocusEffect, + useRovingTabIndex, +} from "@getoutline/react-roving-tabindex"; import { observer } from "mobx-react"; import * as React from "react"; import { Link } from "react-router-dom"; -import { CompositeItem } from "reakit/Composite"; import styled, { css } from "styled-components"; import breakpoint from "styled-components-breakpoint"; import { s, ellipsis } from "@shared/styles"; @@ -34,10 +37,18 @@ function DocumentListItem( ) { const { document, highlight, context, shareId, ...rest } = props; + let itemRef: React.Ref = + React.useRef(null); + if (ref) { + itemRef = ref; + } + + const { focused, ...rovingTabIndex } = useRovingTabIndex(itemRef, false); + useFocusEffect(focused, itemRef); + return ( - { + if (rest.onClick) { + rest.onClick(ev); + } + rovingTabIndex.onClick(ev); + }} > @@ -66,7 +84,7 @@ function DocumentListItem( /> } - + ); } diff --git a/app/components/SearchPopover.tsx b/app/components/SearchPopover.tsx index 1ce8387b7..80ac4969a 100644 --- a/app/components/SearchPopover.tsx +++ b/app/components/SearchPopover.tsx @@ -206,7 +206,7 @@ function SearchPopover({ shareId }: Props) { {t("No results for {{query}}", { query })} } loading={} - renderItem={(item: SearchResult, index, compositeProps) => ( + renderItem={(item: SearchResult, index) => ( )} /> diff --git a/app/scenes/GroupMembers/components/UserListItem.tsx b/app/scenes/GroupMembers/components/UserListItem.tsx deleted file mode 100644 index eecd5ae59..000000000 --- a/app/scenes/GroupMembers/components/UserListItem.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { observer } from "mobx-react"; -import { PlusIcon } from "outline-icons"; -import * as React from "react"; -import { Trans, useTranslation } from "react-i18next"; -import User from "~/models/User"; -import Avatar from "~/components/Avatar"; -import Badge from "~/components/Badge"; -import Button from "~/components/Button"; -import ListItem from "~/components/List/Item"; -import Time from "~/components/Time"; - -type Props = { - user: User; - canEdit: boolean; - onAdd: () => void; -}; - -const UserListItem = ({ user, onAdd, canEdit }: Props) => { - const { t } = useTranslation(); - - return ( - } - subtitle={ - <> - {user.lastActiveAt ? ( - - Active - ) : ( - t("Never signed in") - )} - {user.isInvited && {t("Invited")}} - {user.isAdmin && {t("Admin")}} - - } - actions={ - canEdit ? ( - - ) : undefined - } - /> - ); -}; - -export default observer(UserListItem); diff --git a/app/scenes/Search/Search.tsx b/app/scenes/Search/Search.tsx index 8e5405fd7..15721d6ca 100644 --- a/app/scenes/Search/Search.tsx +++ b/app/scenes/Search/Search.tsx @@ -53,8 +53,8 @@ function Search(props: Props) { // refs const searchInputRef = React.useRef(null); - const resultListCompositeRef = React.useRef(null); - const recentSearchesCompositeRef = React.useRef(null); + const resultListRef = React.useRef(null); + const recentSearchesRef = React.useRef(null); // filters const query = decodeURIComponentSafe(routeMatch.params.term ?? ""); @@ -178,19 +178,9 @@ function Search(props: Props) { } } - const firstResultItem = ( - resultListCompositeRef.current?.querySelectorAll( - "[href]" - ) as NodeListOf - )?.[0]; + const firstItem = (resultListRef.current?.firstElementChild ?? + recentSearchesRef.current?.firstElementChild) as HTMLAnchorElement; - const firstRecentSearchItem = ( - recentSearchesCompositeRef.current?.querySelectorAll( - "li > [href]" - ) as NodeListOf - )?.[0]; - - const firstItem = firstResultItem ?? firstRecentSearchItem; firstItem?.focus(); } }; @@ -277,11 +267,11 @@ function Search(props: Props) { )} - {(compositeProps) => + {() => data?.length ? data.map((result) => ( )) : null @@ -305,10 +294,7 @@ function Search(props: Props) { ) : documentId || collectionId ? null : ( - + )} diff --git a/app/scenes/Search/components/RecentSearchListItem.tsx b/app/scenes/Search/components/RecentSearchListItem.tsx new file mode 100644 index 000000000..32c719e5f --- /dev/null +++ b/app/scenes/Search/components/RecentSearchListItem.tsx @@ -0,0 +1,92 @@ +import { + useFocusEffect, + useRovingTabIndex, +} from "@getoutline/react-roving-tabindex"; +import { CloseIcon } from "outline-icons"; +import * as React from "react"; +import { useTranslation } from "react-i18next"; +import { Link } from "react-router-dom"; +import styled from "styled-components"; +import { s } from "@shared/styles"; +import type SearchQuery from "~/models/SearchQuery"; +import NudeButton from "~/components/NudeButton"; +import Tooltip from "~/components/Tooltip"; +import { hover } from "~/styles"; +import { searchPath } from "~/utils/routeHelpers"; + +type Props = { + searchQuery: SearchQuery; +}; + +function RecentSearchListItem({ searchQuery }: Props) { + const { t } = useTranslation(); + + const ref = React.useRef(null); + + const { focused, ...rovingTabIndex } = useRovingTabIndex(ref, false); + useFocusEffect(focused, ref); + + return ( + + {searchQuery.query} + + { + ev.preventDefault(); + await searchQuery.delete(); + }} + > + + + + + ); +} + +const RemoveButton = styled(NudeButton)` + opacity: 0; + color: ${s("textTertiary")}; + + &:hover { + color: ${s("text")}; + } +`; + +const RecentSearch = styled(Link)` + display: flex; + justify-content: space-between; + color: ${s("textSecondary")}; + cursor: var(--pointer); + padding: 1px 4px; + border-radius: 4px; + position: relative; + font-size: 14px; + + &:before { + content: "·"; + color: ${s("textTertiary")}; + position: absolute; + left: -8px; + } + + &:focus-visible { + outline: none; + } + + &:focus, + &:${hover} { + color: ${s("text")}; + background: ${s("secondaryBackground")}; + + ${RemoveButton} { + opacity: 1; + } + } +`; + +export default RecentSearchListItem; diff --git a/app/scenes/Search/components/RecentSearches.tsx b/app/scenes/Search/components/RecentSearches.tsx index 6f1059f5c..3c9150d27 100644 --- a/app/scenes/Search/components/RecentSearches.tsx +++ b/app/scenes/Search/components/RecentSearches.tsx @@ -1,18 +1,12 @@ import { observer } from "mobx-react"; -import { CloseIcon } from "outline-icons"; import * as React from "react"; import { useTranslation } from "react-i18next"; -import { Link } from "react-router-dom"; -import { CompositeItem } from "reakit/Composite"; import styled from "styled-components"; import { s } from "@shared/styles"; import ArrowKeyNavigation from "~/components/ArrowKeyNavigation"; import Fade from "~/components/Fade"; -import NudeButton from "~/components/NudeButton"; -import Tooltip from "~/components/Tooltip"; import useStores from "~/hooks/useStores"; -import { hover } from "~/styles"; -import { searchPath } from "~/utils/routeHelpers"; +import RecentSearchListItem from "./RecentSearchListItem"; type Props = { /** Callback when the Escape key is pressed while navigating the list */ @@ -36,39 +30,20 @@ function RecentSearches( const content = searches.recent.length ? ( <> {t("Recent searches")} - - - {(compositeProps) => - searches.recent.map((searchQuery) => ( - - - {searchQuery.query} - - { - ev.preventDefault(); - await searchQuery.delete(); - }} - > - - - - - - )) - } - - + + {() => + searches.recent.map((searchQuery) => ( + + )) + } + ) : null; @@ -83,55 +58,9 @@ const Heading = styled.h2` margin-bottom: 0; `; -const List = styled.ol` +const StyledArrowKeyNavigation = styled(ArrowKeyNavigation)` padding: 0; margin-top: 8px; `; -const ListItem = styled.li` - font-size: 14px; - padding: 0; - list-style: none; - position: relative; - - &:before { - content: "·"; - color: ${s("textTertiary")}; - position: absolute; - left: -8px; - } -`; - -const RemoveButton = styled(NudeButton)` - opacity: 0; - color: ${s("textTertiary")}; - - &:hover { - color: ${s("text")}; - } -`; - -const RecentSearch = styled(Link)` - display: flex; - justify-content: space-between; - color: ${s("textSecondary")}; - cursor: var(--pointer); - padding: 1px 4px; - border-radius: 4px; - - &:focus-visible { - outline: none; - } - - &:focus, - &: ${hover} { - color: ${s("text")}; - background: ${s("secondaryBackground")}; - - ${RemoveButton} { - opacity: 1; - } - } -`; - export default observer(React.forwardRef(RecentSearches)); diff --git a/app/scenes/Settings/components/ShareListItem.tsx b/app/scenes/Settings/components/ShareListItem.tsx deleted file mode 100644 index 58722c57f..000000000 --- a/app/scenes/Settings/components/ShareListItem.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import * as React from "react"; -import { useTranslation } from "react-i18next"; -import Share from "~/models/Share"; -import ListItem from "~/components/List/Item"; -import Time from "~/components/Time"; -import ShareMenu from "~/menus/ShareMenu"; - -type Props = { - share: Share; -}; - -const ShareListItem = ({ share }: Props) => { - const { t } = useTranslation(); - const { lastAccessedAt } = share; - - return ( - - {t("Shared")}