Skip to content

ActionMenu2: Add aria-labelledby to ActionList #1759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 14, 2022
6 changes: 6 additions & 0 deletions .changeset/actionmenu2-aria-labelledby.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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