Skip to content
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

fix(ui-breadcrumb): add and update aria tags in Breadcrumb #1788

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
16 changes: 15 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': 'true'
}
)
return updatedChild
}

componentDidMount() {
this.props.makeStyles?.()
}
Expand All @@ -78,9 +88,13 @@ class Breadcrumb extends Component<BreadcrumbProps> {
maxWidth: `${Math.floor(100 / numChildren)}%`
}
return React.Children.map(children, (child, index) => {
const isLastElement = index === numChildren - 1
const hasHref =
React.isValidElement(child) && (child.props as { href?: string }).href

return (
<li css={styles?.crumb} style={inlineStyle}>
{child}
{isLastElement && hasHref ? this.addAriaCurrent(child) : child}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand we can always put this ARIA tag here.
From https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-current "If the element representing the current page in the breadcrumb was not a link, the aria-current is optional.", lets put it always there. same in mdn's example: https://developer.mozilla.org/en-US/docs/Web/CSS/Layout_cookbook/Breadcrumb_Navigation

{index < numChildren - 1 && (
<IconArrowOpenEndSolid color="auto" css={styles?.separator} />
)}
Expand Down
Loading