From 4865e3ba28a5613111a03b43b782936de58ceae3 Mon Sep 17 00:00:00 2001 From: Aman Harwara Date: Mon, 14 Aug 2023 14:42:33 +0530 Subject: [PATCH] refactor: popover a11y aria attributes --- .../WorkspaceSwitcherOption.tsx | 2 +- .../LockscreenWorkspaceSwitcher.tsx | 2 +- .../ChangeEditor/ChangeEditorButton.tsx | 2 +- .../ChangeEditor/ChangeMultipleButton.tsx | 2 +- .../Header/AddItemMenuButton.tsx | 84 ++++++++++--------- .../Header/ContentListHeader.tsx | 2 +- .../ContentTableView/ContentTableView.tsx | 4 +- .../FileContextMenu/FileOptionsPanel.tsx | 8 +- .../FilePreview/FilePreviewModal.tsx | 2 +- .../FileView/FileViewWithoutProtection.tsx | 2 +- .../Components/Footer/AccountMenuButton.tsx | 2 +- .../Components/Footer/QuickSettingsButton.tsx | 2 +- .../Footer/VaultSelectionButton.tsx | 2 +- .../LinkedItems/LinkedItemsButton.tsx | 2 +- .../LinkedItems/LinkedItemsSectionItem.tsx | 2 +- .../Components/Menu/MenuRadioButtonItem.tsx | 5 +- .../javascripts/Components/Modal/Modal.tsx | 2 +- .../Components/NotesOptions/AddTagOption.tsx | 2 +- .../NotesOptions/ChangeEditorOption.tsx | 2 +- .../Listed/ListedActionsOption.tsx | 2 +- .../NotesOptions/NotesOptionsPanel.tsx | 2 +- .../Popover/MobilePopoverContent.tsx | 1 + .../Components/Popover/Popover.tsx | 32 +++++-- .../Popover/PositionedPopoverContent.tsx | 10 ++- .../javascripts/Components/Popover/Types.ts | 8 +- .../General/SmartViews/EditSmartViewModal.tsx | 2 +- .../Vaults/VaultModal/EditVaultModal.tsx | 2 +- .../HistoryModalDialogContent.tsx | 2 +- .../SmartViewBuilder/AddSmartViewModal.tsx | 2 +- .../ItemSelectionPlugin.tsx | 2 +- .../Vaults/AddToVaultMenuOption.tsx | 2 +- 31 files changed, 110 insertions(+), 88 deletions(-) diff --git a/packages/web/src/javascripts/Components/AccountMenu/WorkspaceSwitcher/WorkspaceSwitcherOption.tsx b/packages/web/src/javascripts/Components/AccountMenu/WorkspaceSwitcher/WorkspaceSwitcherOption.tsx index 15df11f55..96c1467e1 100644 --- a/packages/web/src/javascripts/Components/AccountMenu/WorkspaceSwitcher/WorkspaceSwitcherOption.tsx +++ b/packages/web/src/javascripts/Components/AccountMenu/WorkspaceSwitcher/WorkspaceSwitcherOption.tsx @@ -32,7 +32,7 @@ const WorkspaceSwitcherOption: FunctionComponent = ({ mainApplicationGrou = ({ mainApplication = ({ noteViewController, onCl title="Change note type" togglePopover={toggleMenu} disableClickOutside={isClickOutsideDisabled} - anchorElement={buttonRef.current} + anchorElement={buttonRef} open={isOpen} className="pt-2 md:pt-0" > diff --git a/packages/web/src/javascripts/Components/ChangeEditor/ChangeMultipleButton.tsx b/packages/web/src/javascripts/Components/ChangeEditor/ChangeMultipleButton.tsx index 22f1fc9ea..5b1fc2725 100644 --- a/packages/web/src/javascripts/Components/ChangeEditor/ChangeMultipleButton.tsx +++ b/packages/web/src/javascripts/Components/ChangeEditor/ChangeMultipleButton.tsx @@ -23,7 +23,7 @@ const ChangeMultipleButton = ({ application, notesController }: Props) => { title="Change note type" togglePopover={toggleMenu} disableClickOutside={disableClickOutside} - anchorElement={changeButtonRef.current} + anchorElement={changeButtonRef} open={isChangeMenuOpen} className="pt-2 md:pt-0" > diff --git a/packages/web/src/javascripts/Components/ContentListView/Header/AddItemMenuButton.tsx b/packages/web/src/javascripts/Components/ContentListView/Header/AddItemMenuButton.tsx index 9b07e60bf..6a6f24a86 100644 --- a/packages/web/src/javascripts/Components/ContentListView/Header/AddItemMenuButton.tsx +++ b/packages/web/src/javascripts/Components/ContentListView/Header/AddItemMenuButton.tsx @@ -67,47 +67,49 @@ const AddItemMenuButton = ({ - { - setIsMenuOpen((isOpen) => !isOpen) - }} - side="bottom" - align="center" - className="py-2" - > - - { - addNewItem() - setIsMenuOpen(false) - }} - > - - {addButtonLabel} - - { - setCaptureType('photo') - setIsMenuOpen(false) - }} - > - - Take photo - - { - setCaptureType('video') - setIsMenuOpen(false) - }} - > - - Record video - - - + {canShowMenu && ( + { + setIsMenuOpen((isOpen) => !isOpen) + }} + side="bottom" + align="center" + className="py-2" + > + + { + addNewItem() + setIsMenuOpen(false) + }} + > + + {addButtonLabel} + + { + setCaptureType('photo') + setIsMenuOpen(false) + }} + > + + Take photo + + { + setCaptureType('video') + setIsMenuOpen(false) + }} + > + + Record video + + + + )} diff --git a/packages/web/src/javascripts/Components/ContentListView/Header/ContentListHeader.tsx b/packages/web/src/javascripts/Components/ContentListView/Header/ContentListHeader.tsx index 0602897e4..ab8051656 100644 --- a/packages/web/src/javascripts/Components/ContentListView/Header/ContentListHeader.tsx +++ b/packages/web/src/javascripts/Components/ContentListView/Header/ContentListHeader.tsx @@ -116,7 +116,7 @@ const ContentListHeader = ({ /> { { setContextMenuVisible(false) }} @@ -120,7 +120,7 @@ const ItemLinksCell = ({ item }: { item: DecryptedItemInterface }) => { { setContextMenuVisible(false) }} diff --git a/packages/web/src/javascripts/Components/FileContextMenu/FileOptionsPanel.tsx b/packages/web/src/javascripts/Components/FileContextMenu/FileOptionsPanel.tsx index 6982effe0..c3564fdf6 100644 --- a/packages/web/src/javascripts/Components/FileContextMenu/FileOptionsPanel.tsx +++ b/packages/web/src/javascripts/Components/FileContextMenu/FileOptionsPanel.tsx @@ -19,13 +19,7 @@ const FilesOptionsPanel = ({ itemListController }: Props) => { return ( <> - + { { title="Details" open={isFileInfoPanelOpen} togglePopover={toggleFileInfoPanel} - anchorElement={fileInfoButtonRef.current} + anchorElement={fileInfoButtonRef} side="bottom" align="center" > diff --git a/packages/web/src/javascripts/Components/Footer/AccountMenuButton.tsx b/packages/web/src/javascripts/Components/Footer/AccountMenuButton.tsx index b76351e87..a273b6f33 100644 --- a/packages/web/src/javascripts/Components/Footer/AccountMenuButton.tsx +++ b/packages/web/src/javascripts/Components/Footer/AccountMenuButton.tsx @@ -36,7 +36,7 @@ const AccountMenuButton = ({ hasError, controller, mainApplicationGroup, onClick diff --git a/packages/web/src/javascripts/Components/LinkedItems/LinkedItemsSectionItem.tsx b/packages/web/src/javascripts/Components/LinkedItems/LinkedItemsSectionItem.tsx index c50b0e406..605fc8f5b 100644 --- a/packages/web/src/javascripts/Components/LinkedItems/LinkedItemsSectionItem.tsx +++ b/packages/web/src/javascripts/Components/LinkedItems/LinkedItemsSectionItem.tsx @@ -101,7 +101,7 @@ export const LinkedItemsSectionItem = ({ title="Options" open={isMenuOpen} togglePopover={toggleMenu} - anchorElement={menuButtonRef.current} + anchorElement={menuButtonRef} side="bottom" align="center" className="py-2" diff --git a/packages/web/src/javascripts/Components/Menu/MenuRadioButtonItem.tsx b/packages/web/src/javascripts/Components/Menu/MenuRadioButtonItem.tsx index 0bacfc159..42d3dca0f 100644 --- a/packages/web/src/javascripts/Components/Menu/MenuRadioButtonItem.tsx +++ b/packages/web/src/javascripts/Components/Menu/MenuRadioButtonItem.tsx @@ -8,6 +8,7 @@ import { MouseEventHandler, ReactNode, useCallback, + useRef, useState, } from 'react' import Icon from '../Icon/Icon' @@ -27,12 +28,12 @@ const Tooltip = ({ text }: { text: string }) => { [visible], ) - const [anchorElement, setAnchorElement] = useState(null) + const anchorElement = useRef(null) return (
setVisible(true)} diff --git a/packages/web/src/javascripts/Components/Modal/Modal.tsx b/packages/web/src/javascripts/Components/Modal/Modal.tsx index 264c036ae..9af80f3c4 100644 --- a/packages/web/src/javascripts/Components/Modal/Modal.tsx +++ b/packages/web/src/javascripts/Components/Modal/Modal.tsx @@ -131,7 +131,7 @@ const Modal = ({ setShowAdvanced((show) => !show)} align="start" diff --git a/packages/web/src/javascripts/Components/NotesOptions/AddTagOption.tsx b/packages/web/src/javascripts/Components/NotesOptions/AddTagOption.tsx index 8521aa151..61c2998f3 100644 --- a/packages/web/src/javascripts/Components/NotesOptions/AddTagOption.tsx +++ b/packages/web/src/javascripts/Components/NotesOptions/AddTagOption.tsx @@ -67,7 +67,7 @@ const AddTagOption: FunctionComponent = ({ = ({ applic = ({ application, note, icon title="Note options" disableClickOutside={disableClickOutside} togglePopover={toggleMenu} - anchorElement={buttonRef.current} + anchorElement={buttonRef} open={isOpen} className="select-none pt-2" > diff --git a/packages/web/src/javascripts/Components/Popover/MobilePopoverContent.tsx b/packages/web/src/javascripts/Components/Popover/MobilePopoverContent.tsx index 759a46b50..5586d0b83 100644 --- a/packages/web/src/javascripts/Components/Popover/MobilePopoverContent.tsx +++ b/packages/web/src/javascripts/Components/Popover/MobilePopoverContent.tsx @@ -52,6 +52,7 @@ const MobilePopoverContent = ({
diff --git a/packages/web/src/javascripts/Components/Popover/Popover.tsx b/packages/web/src/javascripts/Components/Popover/Popover.tsx index 7927b9ced..7fa738fc9 100644 --- a/packages/web/src/javascripts/Components/Popover/Popover.tsx +++ b/packages/web/src/javascripts/Components/Popover/Popover.tsx @@ -1,7 +1,6 @@ import { MutuallyExclusiveMediaQueryBreakpoints, useMediaQuery } from '@/Hooks/useMediaQuery' import { useAndroidBackHandler } from '@/NativeMobileWeb/useAndroidBackHandler' -import { UuidGenerator } from '@standardnotes/snjs' -import { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react' +import { createContext, useCallback, useContext, useEffect, useId, useMemo, useState } from 'react' import MobilePopoverContent from './MobilePopoverContent' import PositionedPopoverContent from './PositionedPopoverContent' import { PopoverProps } from './Types' @@ -62,11 +61,11 @@ const PositionedPopoverContentWithAnimation = ( } const Popover = (props: PopoverProps) => { - const popoverId = useRef(UuidGenerator.GenerateUuid()) + const popoverId = useId() const addAndroidBackHandler = useAndroidBackHandler() - useRegisterPopoverToParent(popoverId.current) + useRegisterPopoverToParent(popoverId) const [childPopovers, setChildPopovers] = useState>(new Set()) @@ -106,6 +105,27 @@ const Popover = (props: PopoverProps) => { } }, [addAndroidBackHandler, props, props.open]) + useEffect(() => { + const anchorElement = + props.anchorElement && 'current' in props.anchorElement ? props.anchorElement.current : props.anchorElement + + if (anchorElement) { + anchorElement.setAttribute('aria-haspopup', 'true') + if (props.open) { + anchorElement.setAttribute('aria-expanded', 'true') + } else { + anchorElement.removeAttribute('aria-expanded') + } + } + + return () => { + if (anchorElement) { + anchorElement.removeAttribute('aria-haspopup') + anchorElement.removeAttribute('aria-expanded') + } + } + }, [props.anchorElement, props.open]) + const isMobileScreen = useMediaQuery(MutuallyExclusiveMediaQueryBreakpoints.sm) if (isMobileScreen && !props.disableMobileFullscreenTakeover) { @@ -117,7 +137,7 @@ const Popover = (props: PopoverProps) => { }} title={props.title} className={props.className} - id={popoverId.current} + id={popoverId} > {props.children} @@ -126,7 +146,7 @@ const Popover = (props: PopoverProps) => { return ( - + ) } diff --git a/packages/web/src/javascripts/Components/Popover/PositionedPopoverContent.tsx b/packages/web/src/javascripts/Components/Popover/PositionedPopoverContent.tsx index c56cdb817..16271a3b2 100644 --- a/packages/web/src/javascripts/Components/Popover/PositionedPopoverContent.tsx +++ b/packages/web/src/javascripts/Components/Popover/PositionedPopoverContent.tsx @@ -35,7 +35,8 @@ const PositionedPopoverContent = ({ }: PopoverContentProps) => { const [popoverElement, setPopoverElement] = useState(null) const popoverRect = useAutoElementRect(popoverElement) - const anchorElementRect = useAutoElementRect(anchorElement, { + const resolvedAnchorElement = anchorElement && 'current' in anchorElement ? anchorElement.current : anchorElement + const anchorElementRect = useAutoElementRect(resolvedAnchorElement, { updateOnWindowResize: true, }) const anchorPointRect = DOMRect.fromRect({ @@ -75,7 +76,7 @@ const PositionedPopoverContent = ({ usePopoverCloseOnClickOutside({ popoverElement, - anchorElement, + anchorElement: resolvedAnchorElement, togglePopover, childPopovers, hideOnClickInModal, @@ -122,13 +123,14 @@ const PositionedPopoverContent = ({ } as CSSProperties } ref={mergeRefs([setPopoverElement, addCloseMethod])} + id={'popover/' + id} data-popover={id} onKeyDown={(event) => { if (event.key === KeyboardKey.Escape) { event.stopPropagation() togglePopover?.() - if (anchorElement) { - anchorElement.focus() + if (resolvedAnchorElement) { + resolvedAnchorElement.focus() } } }} diff --git a/packages/web/src/javascripts/Components/Popover/Types.ts b/packages/web/src/javascripts/Components/Popover/Types.ts index 13ea287a5..7a7be97d7 100644 --- a/packages/web/src/javascripts/Components/Popover/Types.ts +++ b/packages/web/src/javascripts/Components/Popover/Types.ts @@ -1,4 +1,4 @@ -import { ReactNode } from 'react' +import { ReactNode, RefObject } from 'react' export type PopoverState = 'closed' | 'positioning' | 'open' @@ -20,8 +20,10 @@ type Point = { y: number } +type AnchorElementOrRef = RefObject | HTMLElement | null + type PopoverAnchorElementProps = { - anchorElement: HTMLElement | null + anchorElement: AnchorElementOrRef anchorPoint?: never } @@ -49,7 +51,7 @@ type CommonPopoverProps = { } export type PopoverContentProps = CommonPopoverProps & { - anchorElement?: HTMLElement | null + anchorElement?: AnchorElementOrRef anchorPoint?: Point childPopovers: Set togglePopover?: () => void diff --git a/packages/web/src/javascripts/Components/Preferences/Panes/General/SmartViews/EditSmartViewModal.tsx b/packages/web/src/javascripts/Components/Preferences/Panes/General/SmartViews/EditSmartViewModal.tsx index 6d6d12ccf..50f9d904e 100644 --- a/packages/web/src/javascripts/Components/Preferences/Panes/General/SmartViews/EditSmartViewModal.tsx +++ b/packages/web/src/javascripts/Components/Preferences/Panes/General/SmartViews/EditSmartViewModal.tsx @@ -117,7 +117,7 @@ const EditSmartViewModal = ({ controller, platform }: Props) => { = ({ onCloseDialog, existingVault { = ({ currentNote }) =