Skip to content

Commit

Permalink
apply pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
rezrah committed Nov 28, 2024
1 parent 3401070 commit 338172c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 34 deletions.
15 changes: 4 additions & 11 deletions packages/react/src/Breadcrumbs/Breadcrumbs.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Breadcrumbs', () => {
})

it('renders selected items correctly into the document', () => {
const {getByText, getAllByRole} = render(
const {getByText, getByRole, getAllByRole} = render(
<Breadcrumbs>
<Breadcrumbs.Item href="/">Resources</Breadcrumbs.Item>
<Breadcrumbs.Item href="/copilot">GitHub Copilot</Breadcrumbs.Item>
Expand All @@ -65,16 +65,9 @@ describe('Breadcrumbs', () => {
</Breadcrumbs>,
)

const breadcrumbLinkEls = getAllByRole('link')
expect(breadcrumbLinkEls).toHaveLength(2)

const item1 = breadcrumbLinkEls[0]
expect(item1.textContent).toBe('Resources')
expect(item1.getAttribute('href')).toBe('/')

const item2 = breadcrumbLinkEls[1]
expect(item2.textContent).toBe('GitHub Copilot')
expect(item2.getAttribute('href')).toBe('/copilot')
expect(getAllByRole('link')).toHaveLength(2)
expect(getByRole('link', {name: 'Resources'})).toHaveAttribute('href', '/')
expect(getByRole('link', {name: 'GitHub Copilot'})).toHaveAttribute('href', '/copilot')

const item3 = getByText('Chat')
expect(item3).toHaveAttribute('aria-current', 'page')
Expand Down
44 changes: 21 additions & 23 deletions packages/react/src/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,27 @@ type ItemProps = {
} & BaseProps<HTMLAnchorElement> &
React.HTMLAttributes<HTMLAnchorElement>

const _Item = forwardRef<HTMLAnchorElement, ItemProps>(
({'aria-current': ariaCurrent, className, children, href, selected, ...rest}, ref) => {
return (
<li className={styles.Breadcrumbs__item}>
{selected ? (
<Text
size="100"
variant="muted"
className={clsx(styles.Breadcrumbs__link, styles['Breadcrumbs__link--selected'], className)}
aria-current={ariaCurrent ? ariaCurrent : 'page'}
{...rest}
>
{children}
</Text>
) : (
<InlineLink href={href} ref={ref} className={clsx(styles.Breadcrumbs__link, className)} {...rest}>
{children}
</InlineLink>
)}
</li>
)
},
)
const _Item = forwardRef<HTMLAnchorElement, ItemProps>(({className, children, href, selected, ...rest}, ref) => {
return (
<li className={styles.Breadcrumbs__item}>
{selected ? (
<Text
size="100"
variant="muted"
className={clsx(styles.Breadcrumbs__link, styles['Breadcrumbs__link--selected'], className)}
aria-current="page"
{...rest}
>
{children}
</Text>
) : (
<InlineLink href={href} ref={ref} className={clsx(styles.Breadcrumbs__link, className)} {...rest}>
{children}
</InlineLink>
)}
</li>
)
})

/**
* Use Breadcrumbs to display the current location within a navigational hierarchy.
Expand Down

0 comments on commit 338172c

Please sign in to comment.