[Breadcrumbs] Add new component #14084

oliviertassinari · 2019-01-04T09:26:56Z

We rarely introduce new components. We don't take it lightly. Here is the main drive: people need.

eps1lon

I really like how they are looking ❤️

Mostly just minor nitpicks about code style/patterns. Only the collapsible breadcrumbs are slightly confusing since there is no way of collapsing them. Only expanding as far as I can tell: https://deploy-preview-14084--material-ui.netlify.com/lab/breadcrumbs/#collapsed-breadcrumbs

packages/material-ui-lab/src/Breadcrumb/Breadcrumb.js

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.js

docs/src/pages/lab/breadcrumbs/CollapsedBreadcrumbs.js

docs/src/pages/lab/breadcrumbs/SimpleBreadcrumbs.js

oliviertassinari · 2019-01-23T11:20:14Z

After much debate about what the correct name for the component should be. I think that @mbrookes and I agreed on Breadcrumbs > Breadcrumb as we talk about a breadcrumb trail and we already have Tabs > Tab. (We are going again Bootstrap that do Breadcrumb > BreadcrumbItem)

updated

mbrookes · 2019-01-24T09:36:44Z

docs/src/pages/lab/breadcrumbs/breadcrumbs.md

+If you have been reading the [overrides documentation page](/customization/overrides/)
+but you are not confident jumping in,
+here is one example of how you can change the badge position.
+


This seems incorrect on two fronts:

Copypasta "badge position"

It isn't an example of customizing Breadcrumbs, it's a custom Breadcrumb component. (In fact the wrapper component is somewhat redundant, the styles could simply be applied to the Chip in the render function).

-here is one example of how you can change the badge position. +here is one example of how you can change the breadcrumb link design.

?

For fear of repeating myself, this isn’t a customisation example in the same sense that the overrides page describes.

eps1lon

For the first custom separator the alignment is a little bit off to the bottom: https://deploy-preview-14084--material-ui.netlify.com/lab/breadcrumbs/#custom-separator

https://deploy-preview-14084--material-ui.netlify.com/lab/breadcrumbs/#simple-breadcrumbs The links should not actually navigate away. The other demos are only fakes that trigger an alert. It should be consistent.

All the links currently have an empty anchor which will cause scroll to the top if clicked.

mbrookes · 2019-01-30T23:42:18Z

docs/src/pages/lab/breadcrumbs/breadcrumbs.md

+## Custom separator
+
+The seperator can be any valid React node.
+In the following examples, we are using `"›"`, `"-"` and


sorry to nitpick, but could we get a colon at the end, i.e. "and:"?

Alternatively, is this an example of "too much documentation"? Would "In the following examples, we are using two string separators, and an SVG icon." suffice?

Are you changing it then? (The comment was marked as resolved, but the markdown is unchanged.

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.d.ts

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.js

mbrookes · 2019-01-31T10:31:37Z

docs/src/pages/lab/breadcrumbs/CollapsedBreadcrumbs.js

+    <Paper className={classes.paper}>
+      <Breadcrumbs maxItems={2} arial-label="Breadcrumb navigation">
+        <Breadcrumb>
+          <Link color="inherit" href="#" onClick={handleClick}>


Do we need both onClick and ‘hrefhere?

Yes, otherwise the page scroll to the top.

Ah, okay. I didn't spot the event.preventDefault(); (was on mobile without reading glasses! :).

This isn't how we've shown href in other demos. I think it's confusing, as it isn't how it would typically be used in production.

mbrookes · 2019-01-31T10:39:15Z

docs/src/pages/lab/breadcrumbs/breadcrumbs.md

+If you have been reading the [overrides documentation page](/customization/overrides/)
+but you are not confident jumping in,
+here is one example of how you can change the badge position.
+


For fear of repeating myself, this isn’t a customisation example in the same sense that the overrides page describes.

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.js

docs/src/pages/lab/breadcrumbs/SimpleBreadcrumbs.js

updated

oliviertassinari · 2019-01-31T14:54:09Z

I'm pretty happy with the current state of the pull request, but I would like to explore the alternative APIs we have before merging it. I think that we should optimize for the react-router use case. I believe most people will use a programmatic logic.

The current implementation

We ~try to match a component with a DOM element.

Link: a
Breadcrumb: li
Breadcrumbs: nav > ol

It's more verbose, it requires different imports. But it's flexible.

import Breadcrumbs from '@material-ui/lab/Breadcrumbs';
import Breadcrumb from '@material-ui/lab/Breadcrumb';
import { Link as RouterLink } from 'react-router-dom';
import Link from '@material-ui/core/Link';

<Breadcrumbs arial-label="Breadcrumb">
  <Breadcrumb>
    <Link component={RouterLink} color="inherit" to="/">
      Home
    </Link>
  </Breadcrumb>
  {pathnames.map((value, index) => {
    const last = index === pathnames.length - 1;
    const to = `/${pathnames.slice(0, index + 1).join('/')}`;
    return last ? (
      <Breadcrumb color="textPrimary" key={to}>
        {breadcrumbNameMap[to]}
      </Breadcrumb>
    ) : (
      <Breadcrumb key={to}>
        <Link component={RouterLink} color="inherit" to={to}>
          {breadcrumbNameMap[to]}
        </Link>
      </Breadcrumb>
    );
  })}
</Breadcrumbs>

It's also the structure used by Ant Design: https://ant.design/components/breadcrumb/#components-breadcrumb-demo-basic. It's important to notice that they consider the list of links a breadcrumb when, on our side, we consider the list of links a breadcrumb trail. Hence the Breadcrumb vs Breadcrumbs wording for the top level component.

Alternative n°1

We make the Breadcrumb component go away. I know @mbrookes we have been discussing a long time around the Breadcrumbs/Breadcrumb vs Breadcrumb/BreadcrumbItem perfect wording. But maybe we just need Breadcrumbs:

import Breadcrumbs from '@material-ui/lab/Breadcrumbs';
import Typography from '@material-ui/core/Typography';
import { Link as RouterLink } from 'react-router-dom';
import Link from '@material-ui/core/Link';

<Breadcrumbs arial-label="Breadcrumb">
  <Link component={RouterLink} color="inherit" to="/">
    Home
  </Link>
  {pathnames.map((value, index) => {
    const last = index === pathnames.length - 1;
    const to = `/${pathnames.slice(0, index + 1).join('/')}`;

    return last ? (
      <Typography color="textPrimary" key={to}>
        {breadcrumbNameMap[to]}
      </Typography>
    ) : (
      <Link key={to} component={RouterLink} color="inherit" to={to}>
        {breadcrumbNameMap[to]}
      </Link>
    );
  })}
</Breadcrumbs>

The Breadcrumbs component is now responsible for the nav > ol > li structure. This approach is leveraging the structure we already depend on for injecting the separators. I'm having a hard time seeing the drawback. Let me know! It might look a bit magic. It's also what I like about this solution, it's elegant.

Alternative n°2

We merge the Link component inside Breadcrumb.

import Breadcrumbs from '@material-ui/lab/Breadcrumbs';
import Breadcrumb from '@material-ui/core/Breadcrumb';
import { Link as RouterLink } from 'react-router-dom';

<Breadcrumbs arial-label="Breadcrumb">
  <Breadcrumb LinkProps={{ to: '/', component: RouterLink }}>
    Home
  </Breadcrumb>
  {pathnames.map((value, index) => {
    const last = index === pathnames.length - 1;
    const to = `/${pathnames.slice(0, index + 1).join('/')}`;

    return last ? (
      <Breadcrumb color="textPrimary" LinkComponent="span" key={to}>
        {breadcrumbNameMap[to]}
      </Breadcrumb>
    ) : (
      <Breadcrumb
        key={to}
        LinkProps={{ to, component: RouterLink }}
      >
        {breadcrumbNameMap[to]}
      </Breadcrumb>
    );
  })}
</Breadcrumbs>

It removes the need for an import. Now, I'm not sure that "hiding" the internal is beneficial. People will have to provide their own routing component either from Next.js, react-router or Gatsby. Very few people can leverage a raw <a> element. I think that it's better to expose the Link component as its creating more awareness of its existence.

Alternative n°3

We accept a simple array property like proposed in this tweet https://twitter.com/brad_frost/status/1090733766950223878.

const breadcrumbs = [
  {
    path: '/',
    component: RouterLink,
    label: 'Home',
  },
  ...pathnames.map((value, index) => {
    const to = `/${pathnames.slice(0, index + 1).join('/')}`;

    return {
      path: to,
      component: RouterLink,
      label: breadcrumbNameMap[to],
    }
  })
]

<Breadcrumbs arial-label="Breadcrumb" breadcrumbs={breadcrumbs} />

This is the best structure an end product. Provide the object, and you are set.
However, because of our audience & target, Material-UI users will eventually ask for more customization capabilities. I can already picture it:

I want to add the schema.org's item attributes for SEO, how should I do it?
I want to change the style of the last breadcrumb, it should be a link not a span, how should I do it?
etc.

Ant Design supports this API for react-router@3, but not react-router@4.
https://ant.design/components/breadcrumb/#components-breadcrumb-demo-router.
I think that we should let our users build this type of high-level components using our lower-level primitives.

@mui-org/core-contributors How do you want to move forward?

eps1lon · 2019-01-31T15:12:50Z

About naming: Since ant design and the WAI-ARIA Authoring Practices speak of Breadcrumb when talking about the full component I would tend to Breadcrumb (and BreadCrumbItem which is obsolete with approach 1).

I like this approach. The previous BreadCrumb component didn't really do much from a usability standpoint. I think that forcing a certain DOM structure (nav > ol > li) is justified here since it guarantees good a11y.
Not a fan of those *Component and *Props props. I think this approach is mainly motivated to reduce the verbosity of the first alternative but the pattern stays the same and it's more convoluted what actual component is used depending on the index.

I think that we should let our users build this type of high-level components using our lower-level primitives.

Agree wholeheartedly with this and would therefore dismiss this approach. If users recognize this pattern in their codebase I would encourage them to use their own abstraction instead of providing it out of the box and allowing customization through to many props.

Side note: We should make use of aria-current="page" in our demos which is recommended in the WAI-ARIA authoring practices.

oliviertassinari · 2019-01-31T15:24:19Z

About naming: Since ant design and the WAI-ARIA Authoring Practices speak of Breadcrumb when talking about the full component I would tend to Breadcrumb (and BreadCrumbItem which is obsolete with approach 1).

@eps1lon I don't agree with this point. We have been benchmarking a lot of implementations and a11y resources. It's 50/50. People do both. The final logic was: a breadcrumb is a single entity. You need multiple breadcrumbs to create a trail. It's like the Tabs/Tab components.

eps1lon · 2019-01-31T15:32:01Z

We have been benchmarking a lot of implementations and a11y resources.

Just for my own education could you add those resources? ant design, bootstrap, wikipedia, WAI-ARIA, semantic ui all use the singular.

In the end it doesn't really matter if we only have a component for the collection not every single item.

oliviertassinari · 2019-01-31T15:35:45Z

We have been benchmarking a lot of implementations and a11y resources.

Could you add those resources?

@eps1lon Sure:

Singular:

https://auth0-cosmos.now.sh/docs/#/component/breadcrumb
https://ant.design/components/breadcrumb/
https://elemefe.github.io/element-react/#/en-US/breadcrumb
https://getbootstrap.com/docs/4.2/components/breadcrumb/
https://www.w3.org/TR/wai-aria-practices/#breadcrumb
https://react-bootstrap.github.io/components/breadcrumb/
https://www.w3.org/TR/wai-aria-practices/examples/breadcrumb/index.html
https://reactstrap.github.io/components/breadcrumbs/#app ( component name is singular, page title plural probably because it showcases multiple usage instances)

Plural:

mbrookes · 2019-01-31T16:41:44Z

Also plural (just as a counterpoint to the other w3.org resources):

eps1lon · 2019-01-31T16:49:53Z

https://www.w3.org/WAI/WCAG20/Techniques/working-examples/G65/ex3.html (arial-label)

I love how they write it in singular in the header and in plural in the label. Naming things is not hard it can straight up drive one insane 🙈

https://en.oxforddictionaries.com/definition/breadcrumb uses plural in the context of the design pattern and uses the singular for a single fragment. Let's stick with the plural.

oliviertassinari · 2019-01-31T18:25:29Z

I have implemented "Alternative n°1".

taschetto · 2019-02-07T16:09:33Z

I'd like to suggest a responsive solution for a breadcrumb, which I'm using in my material-ui application. It seems to me that the Collapsed breadcrumbs isn't suitable to a mobile screen as it expands to its full width.

The following video shows a collapsed breadcrumb which opens a menu to navigate on mobile screen. I hope this is useful. I can share the code if needed.

https://drive.google.com/file/d/15NvZ0RAVWALakBoOHOibXxhp1CYCBQaO/view?usp=sharing

oliviertassinari · 2019-02-10T23:33:28Z

It seems to me that the Collapsed breadcrumbs isn't suitable to a mobile screen as it expands to its full width.

@taschetto The breadcrumbs will wrap into two lines if one line isn't enough with the current implementation. Your version is interesting. Could you share the implementation? :)

Blueprint has an interesting implementation leveraging the resize observer API:

oliviertassinari changed the title ~~Mbrookes lab breadcrumbs~~ [lab] Add a Breadcrumb component Jan 4, 2019

oliviertassinari added new feature New feature or request package: lab Specific to @mui/lab labels Jan 4, 2019

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch from f9e9051 to 089c07a Compare January 4, 2019 09:31

oliviertassinari self-assigned this Jan 5, 2019

eps1lon previously requested changes Jan 14, 2019

View reviewed changes

mbrookes force-pushed the mbrookes-lab-breadcrumbs branch from 089c07a to 1a1e414 Compare January 14, 2019 23:22

mbrookes reviewed Jan 16, 2019

View reviewed changes

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.js Outdated Show resolved Hide resolved

oliviertassinari removed their assignment Jan 16, 2019

oliviertassinari commented Jan 17, 2019

View reviewed changes

docs/src/pages/lab/breadcrumbs/CollapsedBreadcrumbs.js Outdated Show resolved Hide resolved

docs/src/pages/lab/breadcrumbs/CollapsedBreadcrumbs.js Outdated Show resolved Hide resolved

docs/src/pages/lab/breadcrumbs/SimpleBreadcrumbs.js Outdated Show resolved Hide resolved

mbrookes force-pushed the mbrookes-lab-breadcrumbs branch from 3535971 to 55c00e8 Compare January 24, 2019 00:04

This comment has been minimized.

Sign in to view

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch from aefa11d to 37cef19 Compare January 24, 2019 07:43

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch from d7174d7 to a2e2d30 Compare January 24, 2019 10:21

mbrookes reviewed Jan 24, 2019

View reviewed changes

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch 5 times, most recently from f1c729b to c2e9f3d Compare January 28, 2019 22:38

oliviertassinari added the PR: good for merge label Jan 28, 2019

eps1lon previously requested changes Jan 30, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

mbrookes and others added 5 commits January 30, 2019 22:10

WIP Breadcrumbs

bdc3a93

refactor

07437b7

Simplify component prop rendering

ab61b05

Simplify Breadcrumbs, fix prop comment order

b18b9fd

Update for @eps1lon's review

4ac2eb5

mbrookes reviewed Jan 30, 2019

View reviewed changes

eps1lon previously requested changes Jan 31, 2019

View reviewed changes

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.d.ts Outdated Show resolved Hide resolved

packages/material-ui-lab/src/Breadcrumbs/Breadcrumbs.js Outdated Show resolved Hide resolved

mbrookes reviewed Jan 31, 2019

View reviewed changes

matt & seb reviews

cc89470

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch from 50bed00 to cc89470 Compare January 31, 2019 14:00

seb review

b1927e3

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch 3 times, most recently from 8dfa72c to ec1dfba Compare January 31, 2019 18:19

Alternative n°1

e3d87d5

oliviertassinari force-pushed the mbrookes-lab-breadcrumbs branch from ec1dfba to e3d87d5 Compare January 31, 2019 18:25

oliviertassinari merged commit 674c523 into mui:master Feb 1, 2019

oliviertassinari deleted the mbrookes-lab-breadcrumbs branch February 1, 2019 09:34

oliviertassinari mentioned this pull request Feb 3, 2019

[Stepper] Breadcrumb like #13176

Closed

oliviertassinari added the component: breadcrumbs This is the name of the generic UI component, not the React module! label Apr 13, 2020

oliviertassinari mentioned this pull request Sep 6, 2020

Add ResizeObserver abstraction #22519

Open

1 task

oliviertassinari changed the title ~~[lab] Add a Breadcrumb component~~ [Breadcrumb] Add new component Jan 9, 2021

oliviertassinari changed the title ~~[Breadcrumb] Add new component~~ [Breadcrumbs] Add new component Jan 9, 2021

oliviertassinari removed the package: lab Specific to @mui/lab label Jan 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Breadcrumbs] Add new component #14084

[Breadcrumbs] Add new component #14084

oliviertassinari commented Jan 4, 2019 •

edited

Loading

eps1lon left a comment

oliviertassinari commented Jan 23, 2019 •

edited

Loading

This comment has been minimized.

mbrookes Jan 24, 2019

oliviertassinari Jan 24, 2019 •

edited

Loading

oliviertassinari Jan 24, 2019

mbrookes Jan 31, 2019

eps1lon left a comment

This comment has been minimized.

mbrookes Jan 30, 2019

mbrookes Jan 31, 2019 •

edited

Loading

mbrookes Jan 31, 2019

oliviertassinari Jan 31, 2019

mbrookes Jan 31, 2019 •

edited

Loading

mbrookes Jan 31, 2019

oliviertassinari commented Jan 31, 2019 •

edited

Loading

eps1lon commented Jan 31, 2019

oliviertassinari commented Jan 31, 2019 •

edited

Loading

eps1lon commented Jan 31, 2019

oliviertassinari commented Jan 31, 2019 •

edited by eps1lon

Loading

mbrookes commented Jan 31, 2019

eps1lon commented Jan 31, 2019

oliviertassinari commented Jan 31, 2019

taschetto commented Feb 7, 2019

oliviertassinari commented Feb 10, 2019 •

edited

Loading

[Breadcrumbs] Add new component #14084

[Breadcrumbs] Add new component #14084

Conversation

oliviertassinari commented Jan 4, 2019 • edited Loading

eps1lon left a comment

Choose a reason for hiding this comment

oliviertassinari commented Jan 23, 2019 • edited Loading

This comment has been minimized.

mbrookes Jan 24, 2019

Choose a reason for hiding this comment

oliviertassinari Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Jan 24, 2019

Choose a reason for hiding this comment

mbrookes Jan 31, 2019

Choose a reason for hiding this comment

eps1lon left a comment

Choose a reason for hiding this comment

This comment has been minimized.

mbrookes Jan 30, 2019

Choose a reason for hiding this comment

mbrookes Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

mbrookes Jan 31, 2019

Choose a reason for hiding this comment

oliviertassinari Jan 31, 2019

Choose a reason for hiding this comment

mbrookes Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

mbrookes Jan 31, 2019

Choose a reason for hiding this comment

oliviertassinari commented Jan 31, 2019 • edited Loading

The current implementation

Alternative n°1

Alternative n°2

Alternative n°3

eps1lon commented Jan 31, 2019

oliviertassinari commented Jan 31, 2019 • edited Loading

eps1lon commented Jan 31, 2019

oliviertassinari commented Jan 31, 2019 • edited by eps1lon Loading

mbrookes commented Jan 31, 2019

eps1lon commented Jan 31, 2019

oliviertassinari commented Jan 31, 2019

taschetto commented Feb 7, 2019

oliviertassinari commented Feb 10, 2019 • edited Loading

oliviertassinari commented Jan 4, 2019 •

edited

Loading

oliviertassinari commented Jan 23, 2019 •

edited

Loading

oliviertassinari Jan 24, 2019 •

edited

Loading

mbrookes Jan 31, 2019 •

edited

Loading

mbrookes Jan 31, 2019 •

edited

Loading

oliviertassinari commented Jan 31, 2019 •

edited

Loading

oliviertassinari commented Jan 31, 2019 •

edited

Loading

oliviertassinari commented Jan 31, 2019 •

edited by eps1lon

Loading

oliviertassinari commented Feb 10, 2019 •

edited

Loading