feat: New Component: Breadcrumb

[brandonlenz]

Summary

Add USWDS Breadcrumb Component.

Related Issues or PRs

Fixes #337

How To Test

Storybook remains the best way to interact with the component, especially regarding meta data. I would recommend inspecting the elements from storybook to verify:

git checkout --track origin/bl-breadcrumb-337
yarn storybook

There are also several jest tests that cover basic rendering and uswds class application.

Screenshots (from storybook)

---

Default Breadcrumb:

Note: non-wrapping breadcrumbs truncate:

---

Breadcrumb with RDFA Metadata:

---

Wrapping Breadcrumb:

---

Breadcrumb using custom Link components:

---

[suzubara]

this looks like a solid start, thanks for working on this! I want to ask if we should consider a more compositional approach to these components, to allow for more flexibility in case users need to pass other props down. for example something like this:

<BreadcrumbNav listProps={{ vocab: "http://schema.org" }}>
  <BreadcrumbItem><a href="#">Link</a></BreadcrumbItem>
  <BreadcrumbItem current>Current Link</BreadcrumbItem>
</BreadcrumbNav>

with Breadcrumbs taking props to pass to the nav, ol elements, and BreadcrumbItem taking props for the li element. I think we'd also want to make sure to support custom link elements and not just a.

[brandonlenz]

this looks like a solid start, thanks for working on this! I want to ask if we should consider a more compositional approach to these components, to allow for more flexibility in case users need to pass other props down. for example something like this:

<BreadcrumbNav listProps={{ vocab: "http://schema.org" }}>
  <BreadcrumbItem><a href="#">Link</a></BreadcrumbItem>
  <BreadcrumbItem current>Current Link</BreadcrumbItem>
</BreadcrumbNav>

with Breadcrumbs taking props to pass to the nav, ol elements, and BreadcrumbItem taking props for the li element. I think we'd also want to make sure to support custom link elements and not just a.

@suzubara I'm with you on the compositional approach being more flexible. I originally pursued that path; my main reasoning for moving away from it was to be able to apply RDFA meta tags to the child components keeping parity with the uswds example. If we go with a compositional approach we would lose control over adding RDFA meta data to the children (without knowing what types of elements will be received). I imagine going that route, we would not be able to support the withRdfaMetaData prop.

As far as passing in custom links, not just a, we face a similar problem of not being able to enforce the uswds class usa-breadcrumb__link. For that I could introduce a BreadcrumbLink component for standard use, giving us something that looks like this:

<BreadcrumbBar listProps={{ vocab: "http://schema.org" }}>
  <Breadcrumb><BreadcrumbLink href="#">Link 1</BreadcrumbLink></Breadcrumb> //This combination of components could remain in a `LinkingBreadcrumb` component
  <Breadcrumb><CustomLink href="#">Link 2</CustomLink></Breadcrumb>
  <Breadcrumb>Current Link</Breadcrumb>
</BreadcrumbBar>

If there's some other React magic you'd recommend I use to get around these hiccups, please let me know. Curious on your thoughts.

[haworku]

I think we'd also want to make sure to support custom link elements

✅ That should have been part of the initial issue description too as well. Whoops! Whatever solution we go with - let's write story examples in storybook with that use case to show how that would be handled.

[brandonlenz]

I think we'd also want to make sure to support custom link elements

✅ That should have been part of the initial issue description too as well. Whoops! Whatever solution we go with - let's write story examples in storybook with that use case to show how that would be handled.

Thanks @haworku and @suzubara. I'm working towards and interface with out-of-the-box components that directly mirror uswds implementations that looks like this:

<BreadcrumbBar>
  <LinkingBreadcrumb href="#">Home</LinkingBreadcrumb>
  <LinkingBreadcrumb href="#">Federal Contracting</LinkingBreadcrumb>
  <LinkingBreadcrumb href="#">Contacting assistance programs</LinkingBreadcrumb>
  <Breadcrumb current>Women-owned small business federal contracting program</Breadcrumb>
</BreadcrumbBar>

The Breadcrumb component itself will be flexible, allowing custom child elements to be passed through (custom links). This means the onus for handling meta data will be entirely on the implementor, but I'll provide props to allow RDFA props to be passed in manually (and a storybook example for how to do so), so that developers can go that route if they so choose. For JSON-LD the onus is on the implementing developer anyways, so no changes there.

PR to be updated shortly.

[suzubara]

this is looking really good! I have 2 main pieces of feedback:

1. for components that pass intrinsic props onto one element, we've been following a pattern of using intersection types to add those props to the component (as opposed to nesting them inside of another prop). So for example, the Breadcrumb component would look like:

export const Breadcrumb = (props: BreadcrumbProps & JSX.IntrinsicElements['li']

I know for components that are passing intrinsic props to multiple elements, they need to be grouped together, but unless necessary I would suggest following the above pattern.

2. I am kind of torn on the use of the LinkingBreadcrumb component over using Breadcrumb and BreadcrumbLink outright.. I realize it's packaging those two components together, but the need to pass separate sets of props for the li and the a smells like configuration to me. Also it enforces a structure where all of the children are rendered inside the a, which would not facilitate the USWDS example with RDFa metadata where the <meta> tag is a sibling of <a>. My suggestion would be to:

• forgo LinkingBreadcrumb or at least recommend it for only the most simplest of use cases where minimal customization is needed. I might also rename it to BreadcrumbWithLink to be more explicit about what it's doing
• modify BreadcrumbLink so that it mirrors the existing Link component, applies the usa-breadcrumb__link but can also handle a custom element
• resulting in an example implementation like:

   <BreadcrumbBar listProps={{ ...rdfaMetadata.ol }}>
      <Breadcrumb { ...rdfaMetadata.li }>
        <BreadcrumbLink href="#" { ...rdfaMetadata.a }>
          <span property="name">Home</span>
        </BreadcrumbLink>
        <meta property="position" content="1" />
      </Breadcrumb>
      <Breadcrumb { ...rdfaMetadata.li }>
        <BreadcrumbLink<CustomLinkProps> to="#" asCustom={CustomLink} { ...rdfaMetadata.a }>
          <span property="name">Federal Contracting</span>
        </BreadcrumbLink>
        <meta property="position" content="2" />
      </Breadcrumb>
      <Breadcrumb current { ...rdfaMetadata.li }>
        <span property="name">
          Women-owned small business federal contracting program
        </span>
        <meta property="position" content="4" />
      </Breadcrumb>
    </BreadcrumbBar>

[haworku]

Things look good to me!

A thought about a way to cut scope for this PR regarding<LinkingBreadcrumb />:
This component is a nicety, not the core behavior of breadcrumb. Consumers could adapt their own version suited to their use case. They know what types of links they have and exactly what attributes they need.

If at the end of the day we feel weirdness or uncertainty about this component, or if refactoring things to be more like <Link> feels like a heavy lift, I could see us moving this pattern into the Storybook examples, rather than exporting from the lib at this point. We could backlog a story about making a more superpowered <BreadcrumbLink> for a later date. Not trying to dissuade you @brandonlenz at all, just want to say I feel like we have the core work for Breadcrumb done (which is great!).

[brandonlenz]

I've now got BreadcrumbLink wrapping Link. I did not want to re-implement any logic from the Link component, to prevent duplicated maintenance in the event the Link implementation were to change. Ultimately I had to repeat some as you can see here.

I was hoping there would be a way to wrap the Link component's signature completely in more of a HOC fashion, and simply inject the className. Due to Link's clever overloading involving the optional generic, I had to mirror at least that part here, making my preferred solution impossible as far as I can tell.

[suzubara]

OK, I have verified the markup matches the examples in the USWDS docs and tested this component out on MilMove! everything works as expected. We should remove the stray eslint-disable lines, as I don't think they are needed, but after that I think this is good to 🚢 🎉