Skip to content

Fix pagelayout stories axe violations #3012

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 18 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e5ccc49
Adding tabIndex to pane and content in CustomStickyHeader story to fi…
radglob Mar 10, 2023
871d6f9
Merge branch 'main' of github.com:primer/react into fix-pagelayout-st…
radglob Mar 10, 2023
00719b1
Merge branch 'main' of github.com:primer/react into fix-pagelayout-st…
radglob Mar 13, 2023
85952d1
Merge commit 'FETCH_HeAD' into fix-pagelayout-stories-axe-violations
radglob Mar 14, 2023
81b73e2
Add role=region, tabIndex and required aria-label when pane overflows.
radglob Mar 15, 2023
2f9121d
Merge branch 'main' of github.com:primer/react into fix-pagelayout-st…
radglob Mar 15, 2023
84a46cf
Fix theme-preval snapshot.
radglob Mar 15, 2023
1d3a7b6
Create wild-snakes-fetch.md
radglob Mar 15, 2023
0432388
Add additional aria-labels to fix errors in other PageLayout stories.
radglob Mar 15, 2023
bc86087
Update docs.
radglob Mar 15, 2023
5076e18
Update generated/components.json
radglob Mar 15, 2023
d476913
Merge branch 'main' into fix-pagelayout-stories-axe-violations
radglob Mar 15, 2023
cfbea70
Merge branch 'main' into fix-pagelayout-stories-axe-violations
radglob Mar 16, 2023
8894628
Merge branch 'main' into fix-pagelayout-stories-axe-violations
radglob Mar 16, 2023
49a99ad
Merge branch 'main' into fix-pagelayout-stories-axe-violations
radglob Mar 16, 2023
ea787f2
Merge branch 'main' into fix-pagelayout-stories-axe-violations
radglob Mar 16, 2023
90883a5
Merge branch 'main' into fix-pagelayout-stories-axe-violations
radglob Mar 17, 2023
bd0e547
Add useOverflow hook, fix broken story missing aria-label.
radglob Mar 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}