Skip to content

Commit

Permalink
Fix pagelayout stories axe violations (#3012)
Browse files Browse the repository at this point in the history
* Adding tabIndex to pane and content in CustomStickyHeader story to fix scrollable region issue.

* Add role=region, tabIndex and required aria-label when pane overflows.

* Fix theme-preval snapshot.

* Create wild-snakes-fetch.md

* Add additional aria-labels to fix errors in other PageLayout stories.

* Update docs.

* Update generated/components.json

* Add useOverflow hook, fix broken story missing aria-label.

---------

Co-authored-by: radglob <radglob@users.noreply.github.com>
  • Loading branch information
radglob and radglob committed Mar 17, 2023
1 parent 560135a commit 04d9db0
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/wild-snakes-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": minor
---

Adds `tabIndex` and `role="region"` to `PageLayout.Pane` when overflow is detected (scrollHeight > clientHeight). Also requires either `aria-labelledby` or `aria-label` when overflow is detected, and throws an error if neither is defined.
10 changes: 10 additions & 0 deletions generated/components.json
Original file line number Diff line number Diff line change
Expand Up @@ -2940,6 +2940,16 @@
{
"name": "PageLayout.Pane",
"props": [
{
"name": "aria-label",
"type": "string | undefined",
"description": "Label for the pane. Required if the pane overflows and doesn't have aria-labelledby."
},
{
"name": "aria-labelledby",
"type": "string | undefined",
"description": "Id of an element that acts as a label for the pane. Required if the pane overflows and doesn't have aria-label."
},
{
"name": "position",
"type": "| 'start' | 'end' | { narrow?: | 'start' | 'end' regular?: | 'start' | 'end' wide?: | 'start' | 'end' }",
Expand Down
12 changes: 11 additions & 1 deletion src/PageLayout/PageLayout.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,16 @@
{
"name": "PageLayout.Pane",
"props": [
{
"name": "aria-label",
"type": "string | undefined",
"description": "Label for the pane. Required if the pane overflows and doesn't have aria-labelledby."
},
{
"name": "aria-labelledby",
"type": "string | undefined",
"description": "Id of an element that acts as a label for the pane. Required if the pane overflows and doesn't have aria-label."
},
{
"name": "position",
"type": "| 'start' | 'end' | { narrow?: | 'start' | 'end' regular?: | 'start' | 'end' wide?: | 'start' | 'end' }",
Expand Down Expand Up @@ -248,4 +258,4 @@
]
}
]
}
}
22 changes: 18 additions & 4 deletions src/PageLayout/PageLayout.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,14 @@ export const StickyPane: Story = args => (
})}
</Box>
</PageLayout.Content>
<PageLayout.Pane position="start" resizable padding="normal" divider="line" sticky={args.sticky}>
<PageLayout.Pane
position="start"
resizable
padding="normal"
divider="line"
sticky={args.sticky}
aria-label="Side pane"
>
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => {
const testId = `paragraph${i}`
Expand Down Expand Up @@ -578,7 +585,7 @@ export const NestedScrollContainer: Story = args => (
))}
</Box>
</PageLayout.Content>
<PageLayout.Pane position="start" padding="normal" divider="line" sticky>
<PageLayout.Pane position="start" padding="normal" divider="line" sticky aria-label="Side pane">
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => (
<Box key={i} as="p" sx={{margin: 0}}>
Expand Down Expand Up @@ -654,7 +661,14 @@ export const CustomStickyHeader: Story = args => (
})}
</Box>
</PageLayout.Content>
<PageLayout.Pane position="start" padding="normal" divider="line" sticky offsetHeader={args.offsetHeader}>
<PageLayout.Pane
position="start"
padding="normal"
divider="line"
aria-label="Aside pane"
sticky
offsetHeader={args.offsetHeader}
>
<Box sx={{display: 'grid', gap: 3}}>
{Array.from({length: args.numParagraphsInPane}).map((_, i) => {
const testId = `paragraph${i}`
Expand Down Expand Up @@ -722,7 +736,7 @@ export const ScrollContainerWithinPageLayoutPane: Story = () => (
<Box sx={{overflow: 'auto'}}>
<Placeholder label="Above inner scroll container" height={120} />
<PageLayout rowGap="none" columnGap="none" padding="none" containerWidth="full">
<PageLayout.Pane position="start" padding="normal" divider="line" sticky>
<PageLayout.Pane position="start" padding="normal" divider="line" sticky aria-label="Sticky pane">
<Box sx={{overflow: 'auto'}}>
<PageLayout.Pane padding="normal">
<Placeholder label="Inner scroll container" height={800} />
Expand Down
20 changes: 20 additions & 0 deletions src/PageLayout/PageLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {useSlots} from '../hooks/useSlots'
import {BetterSystemStyleObject, merge, SxProp} from '../sx'
import {Theme} from '../ThemeProvider'
import {canUseDOM} from '../utils/environment'
import {invariant} from '../utils/invariant'
import {useOverflow} from '../utils/useOverflow'
import VisuallyHidden from '../_VisuallyHidden'
import {useStickyPaneHeight} from './useStickyPaneHeight'

Expand Down Expand Up @@ -416,6 +418,7 @@ const Content: React.FC<React.PropsWithChildren<PageLayoutContentProps>> = ({
}) => {
const isHidden = useResponsiveValue(hidden, false)
const {contentTopRef, contentBottomRef} = React.useContext(PageLayoutContext)

return (
<Box
as="main"
Expand Down Expand Up @@ -479,6 +482,8 @@ export type PageLayoutPaneProps = {
* position={{regular: 'start', narrow: 'end'}}
* ```
*/
'aria-labelledby'?: string
'aria-label'?: string
positionWhenNarrow?: 'inherit' | keyof typeof panePositions
width?: keyof typeof paneWidths
resizable?: boolean
Expand Down Expand Up @@ -522,6 +527,8 @@ const defaultPaneWidth = {small: 256, medium: 296, large: 320}
const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayoutPaneProps>>(
(
{
'aria-label': label,
'aria-labelledby': labelledBy,
position: responsivePosition = 'end',
positionWhenNarrow = 'inherit',
width = 'medium',
Expand Down Expand Up @@ -599,6 +606,7 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
const MIN_PANE_WIDTH = 256 // 256px, related to `--pane-min-width CSS var.
const [minPercent, setMinPercent] = React.useState(0)
const [maxPercent, setMaxPercent] = React.useState(0)
const hasOverflow = useOverflow(paneRef)

const measuredRef = React.useCallback(() => {
if (paneRef.current !== null) {
Expand Down Expand Up @@ -644,6 +652,16 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout

const paneId = useId(id)

let labelProp = undefined
if (hasOverflow) {
invariant(label !== undefined || labelledBy !== undefined)
if (labelledBy) {
labelProp = {'aria-labelledby': labelledBy}
} else {
labelProp = {'aria-label': label}
}
}

return (
<Box
ref={measuredRef}
Expand Down Expand Up @@ -731,6 +749,8 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
'--pane-max-width-diff': '959px',
},
})}
{...(hasOverflow && {tabIndex: 0, role: 'region'})}
{...labelProp}
{...(id && {id: paneId})}
>
{resizable && (
Expand Down
24 changes: 24 additions & 0 deletions src/utils/useOverflow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import {useEffect, useState} from 'react'

export function useOverflow<T extends HTMLElement>(ref: React.RefObject<T>) {
const [hasOverflow, setHasOverflow] = useState(false)

useEffect(() => {
if (ref.current === null) return

const observer = new ResizeObserver(entries => {
for (const entry of entries) {
setHasOverflow(
entry.target.scrollHeight > entry.target.clientHeight || entry.target.scrollWidth > entry.target.clientWidth,
)
}
})

observer.observe(ref.current)
return () => {
observer.disconnect()
}
}, [ref])

return hasOverflow
}

0 comments on commit 04d9db0

Please sign in to comment.