Skip to content

Commit

Permalink
ActionMenu2: Add aria-labelledby to ActionList (#1759)
Browse files Browse the repository at this point in the history
* AnchoredOverlay: Add support for external anchorId

* Add aria-labelledby to ActionList inside ActionMenu

* update tests

* Remove explicit "optional"
  • Loading branch information
siddharthkp committed Jan 14, 2022
1 parent 15e9417 commit 493c6ea
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 13 deletions.
6 changes: 6 additions & 0 deletions .changeset/actionmenu2-aria-labelledby.md
@@ -0,0 +1,6 @@
---
'@primer/react': patch
---

AnchoredOverlay: Add support for passing an id to the anchor. Remove unnecessary aria-labelledby on anchor.
ActionMenu v2: Add aria-labelledby for ActionList
5 changes: 5 additions & 0 deletions docs/content/AnchoredOverlay.mdx
Expand Up @@ -81,6 +81,11 @@ See also [Overlay positioning](/Overlay#positioning).
</>
}
/>
<PropsTableRow
name="anchorId"
type="string"
description="An override to the internal id that will be passed to the anchor."
/>
<PropsTableRow
name="side"
type={`| 'inside-top'
Expand Down
1 change: 1 addition & 0 deletions src/ActionList2/ActionListContainerContext.tsx
Expand Up @@ -6,6 +6,7 @@ type ContextProps = {
container?: string
listRole?: string
itemRole?: string
listLabelledBy?: string
// This can be any function, we don't know anything about the arguments
// to be more specific here, this is as good as (...args: any[]) => unknown
// eslint-disable-next-line @typescript-eslint/ban-types
Expand Down
10 changes: 8 additions & 2 deletions src/ActionList2/List.tsx
Expand Up @@ -41,10 +41,16 @@ export const List = React.forwardRef<HTMLUListElement, ListProps>(
}

/** if list is inside a Menu, it will get a role from the Menu */
const {listRole} = React.useContext(ActionListContainerContext)
const {listRole, listLabelledBy} = React.useContext(ActionListContainerContext)

return (
<ListBox sx={merge(styles, sxProp as SxProp)} role={role || listRole} {...props} ref={forwardedRef}>
<ListBox
sx={merge(styles, sxProp as SxProp)}
role={role || listRole}
aria-labelledby={listLabelledBy}
{...props}
ref={forwardedRef}
>
<ListContext.Provider value={{variant, selectionVariant, showDividers}}>{props.children}</ListContext.Provider>
</ListBox>
)
Expand Down
13 changes: 10 additions & 3 deletions src/ActionMenu2.tsx
@@ -1,13 +1,17 @@
import Button, {ButtonProps} from './Button'
import React from 'react'
import {useSSRSafeId} from '@react-aria/ssr'
import {AnchoredOverlay, AnchoredOverlayProps} from './AnchoredOverlay'
import {OverlayProps} from './Overlay'
import {useProvidedRefOrCreate, useProvidedStateOrCreate} from './hooks'
import {Divider} from './ActionList2/Divider'
import {ActionListContainerContext} from './ActionList2/ActionListContainerContext'
import {MandateProps} from './utils/types'

type MenuContextProps = Pick<AnchoredOverlayProps, 'anchorRef' | 'renderAnchor' | 'open' | 'onOpen' | 'onClose'>
type MenuContextProps = Pick<
AnchoredOverlayProps,
'anchorRef' | 'renderAnchor' | 'open' | 'onOpen' | 'onClose' | 'anchorId'
>
const MenuContext = React.createContext<MenuContextProps>({renderAnchor: null, open: false})

export type ActionMenuProps = {
Expand Down Expand Up @@ -38,6 +42,7 @@ const Menu: React.FC<ActionMenuProps> = ({
const onClose = React.useCallback(() => setCombinedOpenState(false), [setCombinedOpenState])

const anchorRef = useProvidedRefOrCreate(externalAnchorRef)
const anchorId = useSSRSafeId()
let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null

// 🚨 Hack for good API!
Expand All @@ -52,7 +57,7 @@ const Menu: React.FC<ActionMenuProps> = ({
})

return (
<MenuContext.Provider value={{anchorRef, renderAnchor, open: combinedOpenState, onOpen, onClose}}>
<MenuContext.Provider value={{anchorRef, renderAnchor, anchorId, open: combinedOpenState, onOpen, onClose}}>
{contents}
</MenuContext.Provider>
)
Expand Down Expand Up @@ -84,7 +89,7 @@ type MenuOverlayProps = Partial<OverlayProps> & {
const Overlay: React.FC<MenuOverlayProps> = ({children, ...overlayProps}) => {
// we typecast anchorRef as required instead of optional
// because we know that we're setting it in context in Menu
const {anchorRef, renderAnchor, open, onOpen, onClose} = React.useContext(MenuContext) as MandateProps<
const {anchorRef, renderAnchor, anchorId, open, onOpen, onClose} = React.useContext(MenuContext) as MandateProps<
MenuContextProps,
'anchorRef'
>
Expand All @@ -93,6 +98,7 @@ const Overlay: React.FC<MenuOverlayProps> = ({children, ...overlayProps}) => {
<AnchoredOverlay
anchorRef={anchorRef}
renderAnchor={renderAnchor}
anchorId={anchorId}
open={open}
onOpen={onOpen}
onClose={onClose}
Expand All @@ -103,6 +109,7 @@ const Overlay: React.FC<MenuOverlayProps> = ({children, ...overlayProps}) => {
container: 'ActionMenu',
listRole: 'menu',
itemRole: 'menuitem',
listLabelledBy: anchorId,
afterSelect: onClose
}}
>
Expand Down
13 changes: 11 additions & 2 deletions src/AnchoredOverlay/AnchoredOverlay.tsx
Expand Up @@ -17,6 +17,11 @@ interface AnchoredOverlayPropsWithAnchor {
* An override to the internal ref that will be spread on to the renderAnchor
*/
anchorRef?: React.RefObject<HTMLElement>

/**
* An override to the internal id that will be spread on to the renderAnchor
*/
anchorId?: string
}

interface AnchoredOverlayPropsWithoutAnchor {
Expand All @@ -31,6 +36,10 @@ interface AnchoredOverlayPropsWithoutAnchor {
* When renderAnchor is null this can be used to make an anchor that is detached from ActionMenu.
*/
anchorRef: React.RefObject<HTMLElement>
/**
* An override to the internal id that will be spread on to the renderAnchor
*/
anchorId?: string
}

export type AnchoredOverlayWrapperAnchorProps =
Expand Down Expand Up @@ -80,6 +89,7 @@ export type AnchoredOverlayProps = AnchoredOverlayBaseProps &
export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
renderAnchor,
anchorRef: externalAnchorRef,
anchorId: externalAnchorId,
children,
open,
onOpen,
Expand All @@ -94,7 +104,7 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
}) => {
const anchorRef = useProvidedRefOrCreate(externalAnchorRef)
const [overlayRef, updateOverlayRef] = useRenderForcingRef<HTMLDivElement>()
const anchorId = useSSRSafeId()
const anchorId = useSSRSafeId(externalAnchorId)

const onClickOutside = useCallback(() => onClose?.('click-outside'), [onClose])
const onEscape = useCallback(() => onClose?.('escape'), [onClose])
Expand Down Expand Up @@ -154,7 +164,6 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
renderAnchor({
ref: anchorRef,
id: anchorId,
'aria-labelledby': anchorId,
'aria-haspopup': 'true',
'aria-expanded': open,
tabIndex: 0,
Expand Down
1 change: 0 additions & 1 deletion src/__tests__/__snapshots__/ActionMenu.test.tsx.snap
Expand Up @@ -71,7 +71,6 @@ exports[`ActionMenu renders consistently 1`] = `
aria-expanded={false}
aria-haspopup="true"
aria-label="menu"
aria-labelledby="react-aria-1"
className="c0"
id="react-aria-1"
onClick={[Function]}
Expand Down
1 change: 0 additions & 1 deletion src/__tests__/__snapshots__/ActionMenu2.test.tsx.snap
Expand Up @@ -82,7 +82,6 @@ exports[`ActionMenu renders consistently 1`] = `
<button
aria-expanded={false}
aria-haspopup="true"
aria-labelledby="react-aria-1"
className="c1"
id="react-aria-1"
onClick={[Function]}
Expand Down
2 changes: 0 additions & 2 deletions src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap
Expand Up @@ -82,7 +82,6 @@ exports[`AnchoredOverlay renders consistently 1`] = `
<button
aria-expanded={false}
aria-haspopup="true"
aria-labelledby="react-aria-1"
className="c1"
id="react-aria-1"
onClick={[Function]}
Expand Down Expand Up @@ -196,7 +195,6 @@ exports[`AnchoredOverlay should render consistently when open 1`] = `
<button
aria-expanded="true"
aria-haspopup="true"
aria-labelledby="react-aria-1"
class="c1"
id="react-aria-1"
tabindex="0"
Expand Down
1 change: 0 additions & 1 deletion src/__tests__/__snapshots__/DropdownMenu.test.tsx.snap
Expand Up @@ -74,7 +74,6 @@ exports[`DropdownMenu renders consistently 1`] = `
<button
aria-expanded={false}
aria-haspopup="true"
aria-labelledby="react-aria-1"
className="c0"
id="react-aria-1"
onClick={[Function]}
Expand Down
1 change: 0 additions & 1 deletion src/__tests__/__snapshots__/SelectPanel.test.tsx.snap
Expand Up @@ -86,7 +86,6 @@ exports[`SelectPanel renders consistently 1`] = `
<button
aria-expanded={false}
aria-haspopup="true"
aria-labelledby="react-aria-1"
className="c1"
id="react-aria-1"
onClick={[Function]}
Expand Down

0 comments on commit 493c6ea

Please sign in to comment.