Skip to content

Commit

Permalink
fix(ui-breadcrumb): add and update aria tags in Breadcrumb and in doc…
Browse files Browse the repository at this point in the history
…umentation

Closes: INSTUI-4268
  • Loading branch information
ToMESSKa committed Dec 19, 2024
1 parent 295bcb6 commit 99fae9c
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 9 deletions.
12 changes: 10 additions & 2 deletions packages/ui-breadcrumb/src/Breadcrumb/BreadcrumbLink/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,15 @@ class BreadcrumbLink extends Component<BreadcrumbLinkProps> {
}

render() {
const { children, href, renderIcon, iconPlacement, onClick, onMouseEnter } =
this.props
const {
children,
href,
renderIcon,
iconPlacement,
onClick,
onMouseEnter,
isCurrentPage
} = this.props

const props = omitProps(this.props, BreadcrumbLink.allowedProps)

Expand All @@ -69,6 +76,7 @@ class BreadcrumbLink extends Component<BreadcrumbLinkProps> {
onMouseEnter={onMouseEnter}
isWithinText={false}
elementRef={this.handleRef}
{...(isCurrentPage && { 'aria-current': 'page' })}
>
<TruncateText>{children}</TruncateText>
</Link>
Expand Down
11 changes: 9 additions & 2 deletions packages/ui-breadcrumb/src/Breadcrumb/BreadcrumbLink/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ type BreadcrumbLinkOwnProps = {
* Place the icon before or after the text in the Breadcrumb.Link
*/
iconPlacement?: 'start' | 'end'
/**
* Whether the page this breadcrumb points to is the current one. If true, it sets aria-current="page".
* If this prop is not set to true on any breadcrumb element, the one recieving the aria-current="page" will always be the last element, unless the last element's isCurrentPage prop is explicity set to false.
*/
isCurrentPage?: boolean
}

type PropKeys = keyof BreadcrumbLinkOwnProps
Expand All @@ -89,7 +94,8 @@ const propTypes: PropValidators<PropKeys> = {
onMouseEnter: PropTypes.func,
size: PropTypes.oneOf(['small', 'medium', 'large']),
renderIcon: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
iconPlacement: PropTypes.oneOf(['start', 'end'])
iconPlacement: PropTypes.oneOf(['start', 'end']),
isCurrentPage: PropTypes.bool
}

const allowedProps: AllowedPropKeys = [
Expand All @@ -99,7 +105,8 @@ const allowedProps: AllowedPropKeys = [
'onClick',
'onMouseEnter',
'renderIcon',
'size'
'size',
'isCurrentPage'
]

export type { BreadcrumbLinkProps }
Expand Down
21 changes: 17 additions & 4 deletions packages/ui-breadcrumb/src/Breadcrumb/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type: example
{(props, matches) => {
if (matches.includes('tablet')) {
return (
<Breadcrumb label="You are here:">
<Breadcrumb label="breadcrumb">
<Breadcrumb.Link href="#">Student Forecast</Breadcrumb.Link>
<Breadcrumb.Link href="#">University of Utah</Breadcrumb.Link>
<Breadcrumb.Link href="#">University of Utah Colleges</Breadcrumb.Link>
Expand Down Expand Up @@ -52,7 +52,7 @@ Change the `size` prop to control the font-size of the breadcrumbs (default is `
type: example
---
<div>
<Breadcrumb size="small" label="You are here:" margin="none none medium">
<Breadcrumb size="small" label="breadcrumb" margin="none none medium">
<Breadcrumb.Link href="https://instructure.github.io/instructure-ui/">English 204</Breadcrumb.Link>
<Breadcrumb.Link
onClick={function () {
Expand All @@ -65,7 +65,7 @@ type: example
<Breadcrumb.Link>Rabbit Is Rich</Breadcrumb.Link>
</Breadcrumb>
<View as="div" width="40rem">
<Breadcrumb label="You are here:" margin="none none medium">
<Breadcrumb label="breadcrumb" margin="none none medium">
<Breadcrumb.Link href="https://instructure.github.io/instructure-ui/">English 204</Breadcrumb.Link>
<Breadcrumb.Link
onClick={function () {
Expand All @@ -78,7 +78,7 @@ type: example
<Breadcrumb.Link>Rabbit Is Rich</Breadcrumb.Link>
</Breadcrumb>
</View>
<Breadcrumb size="large" label="You are here:">
<Breadcrumb size="large" label="breadcrumb">
<Breadcrumb.Link href="https://instructure.github.io/instructure-ui/">English 204</Breadcrumb.Link>
<Breadcrumb.Link
onClick={function () {
Expand Down Expand Up @@ -126,3 +126,16 @@ type: embed
</Figure>
</Guidelines>
```

```js
---
type: embed
---
<Guidelines>
<Figure recommendation="a11y" title="Accessibility">
<Figure.Item>
To indicate the current element within a breadcrumb, the <a href="https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-current">aria-current</a> attribute is used. In this component, aria-current="page" will automatically be applied to the last element, and we recommend that the current page always be the last element in the breadcrumb. If the last element is not the current page, the isCurrentPage property should be applied to the relevant Breadcrumb.Link to ensure compatibility with screen readers.
</Figure.Item>
</Figure>
</Guidelines>
```
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,68 @@ describe('<Breadcrumb />', () => {
expect(icon).toBeInTheDocument()
expect(icon).toHaveAttribute('aria-hidden', 'true')
})

it('should add aria-current="page" to the last element by default', () => {
const { container } = render(
<Breadcrumb label={TEST_LABEL}>
<Breadcrumb.Link href={TEST_LINK}>{TEST_TEXT_01}</Breadcrumb.Link>
<Breadcrumb.Link>{TEST_TEXT_02}</Breadcrumb.Link>
</Breadcrumb>
)
const links = container.querySelectorAll('[class$="--block-link"]')
const firstLink = links[0]
const lastLink = links[links.length - 1]

expect(firstLink).not.toHaveAttribute('aria-current', 'page')
expect(lastLink).toHaveAttribute('aria-current', 'page')
})

it('should add aria-current="page" to the element if isCurrent is true', () => {
const { container } = render(
<Breadcrumb label={TEST_LABEL}>
<Breadcrumb.Link isCurrentPage href={TEST_LINK}>
{TEST_TEXT_01}
</Breadcrumb.Link>
<Breadcrumb.Link>{TEST_TEXT_02}</Breadcrumb.Link>
</Breadcrumb>
)
const links = container.querySelectorAll('[class$="--block-link"]')
const firstLink = links[0]
const lastLink = links[links.length - 1]

expect(firstLink).toHaveAttribute('aria-current', 'page')
expect(lastLink).not.toHaveAttribute('aria-current', 'page')
})

it('should throw a warning when multiple elements have isCurrent set to true ', () => {
render(
<Breadcrumb label={TEST_LABEL}>
<Breadcrumb.Link isCurrentPage href={TEST_LINK}>
{TEST_TEXT_01}
</Breadcrumb.Link>
<Breadcrumb.Link isCurrentPage>{TEST_TEXT_02}</Breadcrumb.Link>
</Breadcrumb>
)

expect(consoleWarningMock).toHaveBeenCalledWith(
expect.stringContaining(
'Warning: Multiple elements with isCurrentPage=true found. Only one element should be set to current.'
)
)
})

it('should not add aria-current="page" to the last element if it set to false', () => {
const { container } = render(
<Breadcrumb label={TEST_LABEL}>
<Breadcrumb.Link href={TEST_LINK}>{TEST_TEXT_01}</Breadcrumb.Link>
<Breadcrumb.Link isCurrentPage={false}>{TEST_TEXT_02}</Breadcrumb.Link>
</Breadcrumb>
)
const links = container.querySelectorAll('[class$="--block-link"]')
const firstLink = links[0]
const lastLink = links[links.length - 1]

expect(firstLink).not.toHaveAttribute('aria-current', 'page')
expect(lastLink).not.toHaveAttribute('aria-current', 'page')
})
})
30 changes: 29 additions & 1 deletion packages/ui-breadcrumb/src/Breadcrumb/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ class Breadcrumb extends Component<BreadcrumbProps> {
this.ref = el
}

addAriaCurrent = (child: React.ReactNode) => {
const updatedChild = React.cloneElement(
child as React.ReactElement<{ 'aria-current'?: string }>,
{
'aria-current': 'page'
}
)
return updatedChild
}

componentDidMount() {
this.props.makeStyles?.()
}
Expand All @@ -77,10 +87,28 @@ class Breadcrumb extends Component<BreadcrumbProps> {
const inlineStyle = {
maxWidth: `${Math.floor(100 / numChildren)}%`
}
let isAriaCurrentSet = false

return React.Children.map(children, (child, index) => {
const isLastElement = index === numChildren - 1
if (React.isValidElement(child)) {
const isCurrentPage = child.props.isCurrentPage || false
if (isAriaCurrentSet && isCurrentPage) {
console.warn(
`Warning: Multiple elements with isCurrentPage=true found. Only one element should be set to current.`
)
}
if (isCurrentPage) {
isAriaCurrentSet = true
}
}
return (
<li css={styles?.crumb} style={inlineStyle}>
{child}
{!isAriaCurrentSet &&
isLastElement &&
(child as React.ReactElement).props.isCurrentPage !== false
? this.addAriaCurrent(child)
: child}
{index < numChildren - 1 && (
<IconArrowOpenEndSolid color="auto" css={styles?.separator} />
)}
Expand Down

0 comments on commit 99fae9c

Please sign in to comment.