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 11, 2024
1 parent 295bcb6 commit d440ccd
Show file tree
Hide file tree
Showing 5 changed files with 116 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, the element recieving the aria-current="page" will always be the last element on the right by default, unless the last last element's isCurrentPage prop is explicity set to false too.
*/
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
8 changes: 4 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
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 d440ccd

Please sign in to comment.