Skip to content

Commit

Permalink
[UnderlineNav] Accessibility Remediations (#2406)
Browse files Browse the repository at this point in the history
* UnderlineNav accessibility remediations

* update tests

* lint and interaction story update

* ariaLabel optional and add a comment

* remove conditional tabindex and find a better way to manage focus

* restyle arrow btns and fix focus issue

* add aria-label type

* aria-label improvements

* add changeset

* arrow button slide in and out behaviour

* bump octicons_react version up

* update snapshots

* update docs

* add docs name
  • Loading branch information
broccolinisoup committed Oct 11, 2022
1 parent 45afa3d commit 96b004b
Show file tree
Hide file tree
Showing 30 changed files with 1,954 additions and 1,632 deletions.
5 changes: 5 additions & 0 deletions .changeset/wet-glasses-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

UnderlineNav2: Address accessibility feedback and re-style arrow buttons for scroll behaviour
18 changes: 10 additions & 8 deletions docs/content/drafts/UnderlineNav2.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {UnderlineNav} from '@primer/react/drafts'
### Simple

```jsx live drafts
<UnderlineNav>
<UnderlineNav aria-label="Repository">
<UnderlineNav.Item selected>Item 1</UnderlineNav.Item>
<UnderlineNav.Item>Item 2</UnderlineNav.Item>
<UnderlineNav.Item>Item 3</UnderlineNav.Item>
Expand All @@ -26,7 +26,7 @@ import {UnderlineNav} from '@primer/react/drafts'
### With Icons

```jsx live drafts
<UnderlineNav>
<UnderlineNav aria-label="Repository">
<UnderlineNav.Item selected icon={CodeIcon}>
Code
</UnderlineNav.Item>
Expand All @@ -53,7 +53,7 @@ When overflow occurs, the component first hides icons if present to optimize for
#### Items Without Icons

```jsx live drafts
<UnderlineNav>
<UnderlineNav aria-label="Repository">
<UnderlineNav.Item selected icon={CodeIcon}>
Code
</UnderlineNav.Item>
Expand Down Expand Up @@ -83,7 +83,7 @@ When overflow occurs, the component first hides icons if present to optimize for
If there is still overflow, the component will behave depending on the pointer.

```jsx live drafts
<UnderlineNav>
<UnderlineNav aria-label="Repository">
<UnderlineNav.Item selected icon={CodeIcon}>
Code
</UnderlineNav.Item>
Expand Down Expand Up @@ -114,7 +114,7 @@ If there is still overflow, the component will behave depending on the pointer.
### Loading state for counters

```jsx live drafts
<UnderlineNav loadingCounters={true}>
<UnderlineNav aria-label="Repository" loadingCounters={true}>
<UnderlineNav.Item counter={4} selected>
Item 1
</UnderlineNav.Item>
Expand All @@ -128,9 +128,11 @@ If there is still overflow, the component will behave depending on the pointer.
### UnderlineNav

<PropsTable>
<PropsTableRow name="aria-label" type="string" />
<PropsTableRow name="aria-labelledby" type="string" />
<PropsTableRow name="aria-describedby" type="string" />
<PropsTableRow
name="aria-label"
type="string"
description="A unique name for the rendered 'nav' landmark. It will also be used to label the arrow buttons that control the scroll behaviour on coarse pointer devices. (I.e. 'Scroll ${aria-label} left/right')"
/>
<PropsTableRow
name="loadingCounters"
type="boolean"
Expand Down
15 changes: 8 additions & 7 deletions docs/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
},
"dependencies": {
"@primer/gatsby-theme-doctocat": "^4.2.0",
"@primer/octicons-react": "^17.5.0",
"@primer/octicons-react": "^17.7.0",
"@primer/primitives": "4.1.0",
"@styled-system/prop-types": "^5.1.0",
"@styled-system/theme-get": "^5.1.0",
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"@github/markdown-toolbar-element": "^2.1.0",
"@github/paste-markdown": "^1.4.0",
"@primer/behaviors": "^1.1.1",
"@primer/octicons-react": "^17.3.0",
"@primer/octicons-react": "^17.7.0",
"@primer/primitives": "7.9.0",
"@react-aria/ssr": "^3.1.0",
"@styled-system/css": "^5.1.5",
Expand Down
2 changes: 2 additions & 0 deletions src/NavList/__snapshots__/NavList.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
aria-hidden="true"
class="c8"
fill="currentColor"
focusable="false"
height="16"
role="img"
style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
Expand Down Expand Up @@ -1702,6 +1703,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
aria-hidden="true"
class="c8"
fill="currentColor"
focusable="false"
height="16"
role="img"
style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
Expand Down
4 changes: 2 additions & 2 deletions src/UnderlineNav2/UnderlineNav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const ResponsiveUnderlineNav = ({
{navigation: 'Security', icon: ShieldLockIcon}
]
return (
<UnderlineNav label="Repository" loadingCounters={loadingCounters}>
<UnderlineNav aria-label="Repository" loadingCounters={loadingCounters}>
{items.map(item => (
<UnderlineNav.Item
key={item.navigation}
Expand Down Expand Up @@ -91,7 +91,7 @@ describe('UnderlineNav', () => {
it('fires onSelect on click and keypress', async () => {
const onSelect = jest.fn()
const {getByText} = render(
<UnderlineNav label="Test nav">
<UnderlineNav aria-label="Test Navigation">
<UnderlineNav.Item onSelect={onSelect}>Item 1</UnderlineNav.Item>
<UnderlineNav.Item onSelect={onSelect}>Item 2</UnderlineNav.Item>
<UnderlineNav.Item onSelect={onSelect}>Item 3</UnderlineNav.Item>
Expand Down
50 changes: 40 additions & 10 deletions src/UnderlineNav2/UnderlineNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import {
moreMenuStyles,
menuItemStyles
} from './styles'
import {LeftArrowButton, RightArrowButton} from './UnderlineNavArrowButton'
import {ArrowButton} from './UnderlineNavArrowButton'
import styled from 'styled-components'
import {LoadingCounter} from './LoadingCounter'

export type UnderlineNavProps = {
label: string
'aria-label'?: React.AriaAttributes['aria-label']
as?: React.ElementType
align?: 'right'
sx?: SxProp
Expand Down Expand Up @@ -68,6 +68,7 @@ const handleArrowBtnsVisibility = (
const scrollOffsets = {scrollLeft, scrollRight}
callback(scrollOffsets)
}

const overflowEffect = (
navWidth: number,
moreMenuWidth: number,
Expand Down Expand Up @@ -157,7 +158,7 @@ export const UnderlineNav = forwardRef(
{
as = 'nav',
align,
label,
'aria-label': ariaLabel,
sx: sxProp = {},
afterSelect,
variant = 'default',
Expand Down Expand Up @@ -225,6 +226,8 @@ export const UnderlineNav = forwardRef(

const [selectedLink, setSelectedLink] = useState<RefObject<HTMLElement> | undefined>(undefined)

const [focusedLink, setFocusedLink] = useState<RefObject<HTMLElement> | null>(null)

// selectedLinkText is needed to be able set the selected menu item as selectedLink.
// This is needed because setSelectedLink only accepts ref but at the time of setting selected menu item as selectedLink, its ref as a list item is not available
const [selectedLinkText, setSelectedLinkText] = useState<string>('')
Expand Down Expand Up @@ -311,14 +314,28 @@ export const UnderlineNav = forwardRef(
return () => listEl?.removeEventListener('scroll', scrollOnList)
}, [scrollOnList])

useEffect(() => {
// scroll the selected link into the view (coarse pointer behaviour)
if (isCoarsePointer && selectedLink?.current && listRef.current) {
scrollIntoView(selectedLink.current, listRef.current, underlineNavScrollMargins)
function scrollLinkIntoView(link: RefObject<HTMLElement>) {
if (link.current && listRef.current) {
scrollIntoView(link.current, listRef.current, underlineNavScrollMargins)
return
}
}

useEffect(() => {
// scroll the selected link into the view (coarse pointer behaviour)
selectedLink?.current && isCoarsePointer && scrollLinkIntoView(selectedLink)
}, [selectedLink, isCoarsePointer])

useEffect(() => {
// scroll the focused link into the view (coarse pointer behaviour)
focusedLink?.current && isCoarsePointer && scrollLinkIntoView(focusedLink)
}, [focusedLink, isCoarsePointer])

if (!ariaLabel) {
// eslint-disable-next-line no-console
console.warn('Use the `aria-label` prop to provide an accessible label for assistive technology')
}

return (
<UnderlineNavContext.Provider
value={{
Expand All @@ -329,6 +346,7 @@ export const UnderlineNav = forwardRef(
setSelectedLink,
selectedLinkText,
setSelectedLinkText,
setFocusedLink,
selectEvent,
afterSelect: afterSelectHandler,
variant,
Expand All @@ -339,11 +357,17 @@ export const UnderlineNav = forwardRef(
<Box
as={as}
sx={merge<BetterSystemStyleObject>(getNavStyles(theme, {align}), sxProp)}
aria-label={label}
aria-label={ariaLabel}
ref={navRef}
>
{isCoarsePointer && (
<LeftArrowButton show={scrollValues.scrollLeft > 0} onScrollWithButton={onScrollWithButton} />
<ArrowButton
scrollValue={scrollValues.scrollLeft}
type="left"
show={scrollValues.scrollLeft > 0}
onScrollWithButton={onScrollWithButton}
aria-label={ariaLabel}
/>
)}

<NavigationList sx={merge<BetterSystemStyleObject>(responsiveProps.overflowStyles, ulStyles)} ref={listRef}>
Expand Down Expand Up @@ -390,7 +414,13 @@ export const UnderlineNav = forwardRef(
</NavigationList>

{isCoarsePointer && (
<RightArrowButton show={scrollValues.scrollRight > 0} onScrollWithButton={onScrollWithButton} />
<ArrowButton
scrollValue={scrollValues.scrollRight}
type="right"
show={scrollValues.scrollRight > 0}
onScrollWithButton={onScrollWithButton}
aria-label={ariaLabel}
/>
)}
</Box>
</UnderlineNavContext.Provider>
Expand Down
76 changes: 49 additions & 27 deletions src/UnderlineNav2/UnderlineNavArrowButton.tsx
Original file line number Diff line number Diff line change
@@ -1,44 +1,66 @@
import React, {useContext} from 'react'
import {IconButton} from '../Button/IconButton'
import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react'
import {getLeftArrowHiddenBtn, getRightArrowHiddenBtn, getLeftArrowVisibleBtn, getRightArrowVisibleBtn} from './styles'
import {btnWrapperStyles, getArrowBtnStyles} from './styles'
import {OnScrollWithButtonEventType} from './types'
import {UnderlineNavContext} from './UnderlineNavContext'
import Box from '../Box'

const LeftArrowButton = ({
const ArrowButton = ({
scrollValue,
type,
show,
onScrollWithButton
onScrollWithButton,
'aria-label': ariaLabel
}: {
scrollValue: number
type: 'left' | 'right'
show: boolean
onScrollWithButton: OnScrollWithButtonEventType
'aria-label'?: React.AriaAttributes['aria-label']
}) => {
const leftBtnRef = React.useRef<HTMLButtonElement>(null)
const rightBtnRef = React.useRef<HTMLButtonElement>(null)
const {theme} = useContext(UnderlineNavContext)
return (
<IconButton
aria-label="Scroll Left"
onClick={(e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => onScrollWithButton(e, -1)}
icon={ChevronLeftIcon}
sx={show ? getLeftArrowVisibleBtn(theme) : getLeftArrowHiddenBtn(theme)}
/>
)
}
const direction = type === 'left' ? -1 : 1
const ARROW_BTN_WIDTH = 44 // Min touch target size is 44px

// re-trigger focus on the button with aria-disabled=true when it becomes hidden to communicate to screen readers that the button is no longer available
React.useEffect(() => {
const currentBtn = type === 'left' ? leftBtnRef.current : rightBtnRef.current
if (currentBtn?.getAttribute('aria-disabled') === 'true') {
currentBtn.focus()
} else {
// eslint-disable-next-line github/no-blur
currentBtn?.blur()
}
}, [show, type])

let translateX = 0
let display = 'flex'

// Determine the translateX value to transform for the slide in/out effect
if (scrollValue === 0) {
// If the scrollValue is 0, the buttons should be hidden
translateX = ARROW_BTN_WIDTH * direction
// This is mainly needed for the right arrow button. Because hiding translateX value for it is positive (44px) and this positive value was causing button to be visibly overflowed rathan than hiding.
display = 'none'
} else if (scrollValue <= ARROW_BTN_WIDTH) translateX = (ARROW_BTN_WIDTH - scrollValue) * direction
else translateX = 0

const RightArrowButton = ({
show,
onScrollWithButton
}: {
show: boolean
onScrollWithButton: OnScrollWithButtonEventType
}) => {
const {theme} = useContext(UnderlineNavContext)
return (
<IconButton
aria-label="Scroll Right"
onClick={(e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => onScrollWithButton(e, 1)}
icon={ChevronRightIcon}
sx={show ? getRightArrowVisibleBtn(theme) : getRightArrowHiddenBtn(theme)}
/>
<Box sx={btnWrapperStyles(theme, type, show, translateX, display)}>
<IconButton
tabIndex={show ? 0 : -1}
ref={type === 'left' ? leftBtnRef : rightBtnRef}
aria-label={`Scroll ${ariaLabel} navigation ${type}`}
onClick={(e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => onScrollWithButton(e, direction)}
icon={type === 'left' ? ChevronLeftIcon : ChevronRightIcon}
sx={getArrowBtnStyles(theme, type)}
aria-disabled={!show}
/>
</Box>
)
}

export {LeftArrowButton, RightArrowButton}
export {ArrowButton}
Loading

0 comments on commit 96b004b

Please sign in to comment.