Skip to content

Rename and deprecate Breadcrumb to Breadcrumbs #1448

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

Merged
merged 14 commits into from
Sep 23, 2021
Merged
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
5 changes: 5 additions & 0 deletions .changeset/itchy-horses-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Rename `Breadcrumb` component to `Breadcrumbs`
5 changes: 5 additions & 0 deletions .changeset/pretty-walls-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": major
---

Remove `.Breadcrumb` classname from the root element of the `Breadcrumbs` component. This change shouldn't break anything unless you have styles, scripts, or tests that reference the `.Breadcrumb` classname.
24 changes: 12 additions & 12 deletions docs/content/Breadcrumbs.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
title: Breadcrumbs
status: Alpha
source: https://github.com/primer/react/blob/main/src/Breadcrumb.tsx
source: https://github.com/primer/react/blob/main/src/Breadcrumbs.tsx
---

Breadcrumbs are used to show taxonomical context on pages that are many levels deep in a site’s hierarchy. Breadcrumbs show and link to parent, grandparent, and sometimes great-grandparent pages. Breadcrumbs are most appropriate on pages that:
Expand All @@ -10,21 +10,21 @@ Breadcrumbs are used to show taxonomical context on pages that are many levels d
- Do not have a section-level navigation
- May need the ability to quickly go back to the previous (parent) page

To use Breadcrumb with [react-router](https://github.com/ReactTraining/react-router) or
To use Breadcrumbs with [react-router](https://github.com/ReactTraining/react-router) or
[react-router-dom](https://www.npmjs.com/package/react-router-dom), pass
`as={NavLink}` and omit the `selected` prop.
This ensures that the NavLink gets `activeClassName='selected'`

## Default example

```jsx live
<Breadcrumb>
<Breadcrumb.Item href="/">Home</Breadcrumb.Item>
<Breadcrumb.Item href="/about">About</Breadcrumb.Item>
<Breadcrumb.Item href="/about/team" selected>
<Breadcrumbs>
<Breadcrumbs.Item href="/">Home</Breadcrumbs.Item>
<Breadcrumbs.Item href="/about">About</Breadcrumbs.Item>
<Breadcrumbs.Item href="/about/team" selected>
Team
</Breadcrumb.Item>
</Breadcrumb>
</Breadcrumbs.Item>
</Breadcrumbs>
```

## System props
Expand All @@ -35,15 +35,15 @@ System props are deprecated in all components except [Box](/Box). Please use the

</Note>

Breadcrumb and Breadcrumb.Item components get `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.
Breadcrumbs and Breadcrumbs.Item components get `COMMON` system props. Read our [System Props](/system-props) doc page for a full list of available props.

## Component props

### Breadcrumb
### Breadcrumbs

The `Breadcrumb` component does not receive any additional props besides `COMMON` system props.
The `Breadcrumbs` component does not receive any additional props besides `COMMON` system props.

### Breadcrumb.Item
### Breadcrumbs.Item

| Prop name | Type | Default | Description |
| :-------- | :------ | :-----: | :----------------------------------------------- |
Expand Down
40 changes: 27 additions & 13 deletions src/Breadcrumb.tsx → src/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,38 @@ const Wrapper = styled.li`
}
`

const BreadcrumbBase = styled.nav<SystemFlexProps & SystemCommonProps & SxProp>`
const BreadcrumbsBase = styled.nav<SystemFlexProps & SystemCommonProps & SxProp>`
display: flex;
justify-content: space-between;
${COMMON};
${FLEX};
${sx};
`

export type BreadcrumbProps = ComponentProps<typeof BreadcrumbBase>
export type BreadcrumbsProps = ComponentProps<typeof BreadcrumbsBase>

function Breadcrumb({className, children, theme, ...rest}: React.PropsWithChildren<BreadcrumbProps>) {
const classes = classnames(className, 'Breadcrumb')
function Breadcrumbs({className, children, theme, ...rest}: React.PropsWithChildren<BreadcrumbsProps>) {
const wrappedChildren = React.Children.map(children, child => <Wrapper theme={theme}>{child}</Wrapper>)
return (
<BreadcrumbBase className={classes} aria-label="breadcrumb" theme={theme} {...rest}>
<BreadcrumbsBase className={className} aria-label="Breadcrumbs" theme={theme} {...rest}>
<Box as="ol" my={0} pl={0}>
{wrappedChildren}
</Box>
</BreadcrumbBase>
</BreadcrumbsBase>
)
}

type StyledBreadcrumbItemProps = {
type StyledBreadcrumbsItemProps = {
to?: History.LocationDescriptor
selected?: boolean
} & SystemCommonProps &
SxProp

const BreadcrumbItem = styled.a.attrs<StyledBreadcrumbItemProps>(props => ({
const BreadcrumbsItem = styled.a.attrs<StyledBreadcrumbsItemProps>(props => ({
activeClassName: typeof props.to === 'string' ? 'selected' : '',
className: classnames(props.selected && SELECTED_CLASS, props.className),
'aria-current': props.selected ? 'page' : null
}))<StyledBreadcrumbItemProps>`
}))<StyledBreadcrumbsItemProps>`
color: ${get('colors.accent.fg')};
display: inline-block;
font-size: ${get('fontSizes.1')};
Expand All @@ -79,9 +78,24 @@ const BreadcrumbItem = styled.a.attrs<StyledBreadcrumbItemProps>(props => ({
${sx};
`

Breadcrumb.displayName = 'Breadcrumb'
Breadcrumbs.displayName = 'Breadcrumbs'

BreadcrumbItem.displayName = 'Breadcrumb.Item'
BreadcrumbsItem.displayName = 'Breadcrumbs.Item'

export type BreadcrumbItemProps = ComponentProps<typeof BreadcrumbItem>
export default Object.assign(Breadcrumb, {Item: BreadcrumbItem})
export type BreadcrumbsItemProps = ComponentProps<typeof BreadcrumbsItem>
export default Object.assign(Breadcrumbs, {Item: BreadcrumbsItem})

/**
* @deprecated Use the `Breadcrumbs` component instead (i.e. `<Breadcrumb>` → `<Breadcrumbs>`)
*/
export const Breadcrumb = Object.assign(Breadcrumbs, {Item: BreadcrumbsItem})

/**
* @deprecated Use the `BreadcrumbsProps` type instead
*/
export type BreadcrumbProps = ComponentProps<typeof BreadcrumbsBase>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add deprecation warnings for these types as well?

Copy link
Contributor Author

@SferaDev SferaDev Sep 21, 2021

Choose a reason for hiding this comment

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

Sure thing! I followed the same approach as when Position subcomponents where deprecated (component had deprecation warning but not the props types). In any case makes sense to see the message in the IDE if you're importing the outdated types.


/**
* @deprecated Use the `BreadcrumbsItemProps` type instead
*/
export type BreadcrumbItemProps = ComponentProps<typeof BreadcrumbsItem>
31 changes: 0 additions & 31 deletions src/__tests__/Breadcrumb.tsx

This file was deleted.

28 changes: 28 additions & 0 deletions src/__tests__/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React from 'react'
import {Breadcrumbs, Breadcrumb} from '..'
import {render, behavesAsComponent, checkExports} from '../utils/testing'
import {COMMON} from '../constants'
import {render as HTMLRender, cleanup} from '@testing-library/react'
import {axe, toHaveNoViolations} from 'jest-axe'
import 'babel-polyfill'
expect.extend(toHaveNoViolations)

describe('Breadcrumbs', () => {
behavesAsComponent({Component: Breadcrumbs, systemPropArray: [COMMON]})

checkExports('Breadcrumbs', {
default: Breadcrumbs,
Breadcrumb
})

it('should have no axe violations', async () => {
const {container} = HTMLRender(<Breadcrumbs />)
const results = await axe(container)
expect(results).toHaveNoViolations()
cleanup()
})

it('renders a <nav>', () => {
expect(render(<Breadcrumbs />).type).toEqual('nav')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,31 @@ import {cleanup, render as HTMLRender} from '@testing-library/react'
import 'babel-polyfill'
import {axe, toHaveNoViolations} from 'jest-axe'
import React from 'react'
import {Breadcrumb} from '..'
import {Breadcrumbs} from '..'
import {COMMON} from '../constants'
import {behavesAsComponent, render} from '../utils/testing'
expect.extend(toHaveNoViolations)

describe('Breadcrumb.Item', () => {
behavesAsComponent({Component: Breadcrumb.Item, systemPropArray: [COMMON]})
describe('Breadcrumbs.Item', () => {
behavesAsComponent({Component: Breadcrumbs.Item, systemPropArray: [COMMON]})

it('renders an <a> by default', () => {
expect(render(<Breadcrumb.Item />).type).toEqual('a')
expect(render(<Breadcrumbs.Item />).type).toEqual('a')
})

it('should have no axe violations', async () => {
const {container} = HTMLRender(<Breadcrumb.Item />)
const {container} = HTMLRender(<Breadcrumbs.Item />)
const results = await axe(container)
expect(results).toHaveNoViolations()
cleanup()
})

it('respects the "selected" prop', () => {
expect(render(<Breadcrumb.Item selected />)).toMatchSnapshot()
expect(render(<Breadcrumbs.Item selected />)).toMatchSnapshot()
})

it('adds activeClassName={SELECTED_CLASS} when it gets a "to" prop', () => {
const Link = ({theme: _ignoredTheme, ...props}: Record<string, unknown>) => <div {...props} />
expect(render(<Breadcrumb.Item as={Link} to="#" />)).toMatchSnapshot()
expect(render(<Breadcrumbs.Item as={Link} to="#" />)).toMatchSnapshot()
})
})
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Breadcrumb renders consistently 1`] = `
exports[`Breadcrumbs renders consistently 1`] = `
.c1 {
margin-top: 0;
margin-bottom: 0;
Expand All @@ -19,8 +19,8 @@ exports[`Breadcrumb renders consistently 1`] = `
}

<nav
aria-label="breadcrumb"
className="c0 Breadcrumb"
aria-label="Breadcrumbs"
className="c0"
>
<ol
className="c1"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Breadcrumb.Item adds activeClassName={SELECTED_CLASS} when it gets a "to" prop 1`] = `
exports[`Breadcrumbs.Item adds activeClassName={SELECTED_CLASS} when it gets a "to" prop 1`] = `
.c0 {
color: #0969da;
display: inline-block;
Expand All @@ -27,7 +27,7 @@ exports[`Breadcrumb.Item adds activeClassName={SELECTED_CLASS} when it gets a "t
/>
`;

exports[`Breadcrumb.Item renders consistently 1`] = `
exports[`Breadcrumbs.Item renders consistently 1`] = `
.c0 {
color: #0969da;
display: inline-block;
Expand All @@ -52,7 +52,7 @@ exports[`Breadcrumb.Item renders consistently 1`] = `
/>
`;

exports[`Breadcrumb.Item respects the "selected" prop 1`] = `
exports[`Breadcrumbs.Item respects the "selected" prop 1`] = `
.c0 {
color: #0969da;
display: inline-block;
Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export {default as AvatarStack} from './AvatarStack'
export type {AvatarStackProps} from './AvatarStack'
export {default as BranchName} from './BranchName'
export type {BranchNameProps} from './BranchName'
export {default as Breadcrumb} from './Breadcrumb'
export type {BreadcrumbProps, BreadcrumbItemProps} from './Breadcrumb'
export {default as Breadcrumbs, Breadcrumb} from './Breadcrumbs'
export type {BreadcrumbsProps, BreadcrumbsItemProps, BreadcrumbProps, BreadcrumbItemProps} from './Breadcrumbs'
export {
default as Button,
ButtonDanger,
Expand Down