feat(breadcrumbs): Redesign with typed variants, responsive collapse, and a11y#119060
Draft
Jesse-Box wants to merge 9 commits into
Draft
feat(breadcrumbs): Redesign with typed variants, responsive collapse, and a11y#119060Jesse-Box wants to merge 9 commits into
Jesse-Box wants to merge 9 commits into
Conversation
…raphics
Replace the flat `Breadcrumbs` + `Crumb[]` API with a typed discriminated
union `BreadcrumbList` + `items` array. Each breadcrumb slot now has an
explicit variant ('link', 'page-title', 'select-projects') so developers
must consciously choose a crumb type rather than relying on position.
Key changes:
- Add `BreadcrumbList` as the new preferred API with typed `BreadcrumbItem`
discriminated union
- Add `BreadcrumbItemLink`, `BreadcrumbItemPageTitle`,
`BreadcrumbItemSelectProjects`, and `BreadcrumbItemMenuBreadcrumbs`
(internal overflow) item components
- Add `BreadcrumbDividerCombo` (internal) to pair items with slash dividers
- Add container-query overflow: link parent items collapse into a menu
below 800px container width
- Add structured `pagination` prop on `BreadcrumbItemPageTitle` replacing
ReactNode — callers pass {previous, next} items with `to`, `disabled`,
`ariaLabel`, and optional `tooltip`; chevron buttons rendered internally
- Extract `LeadingGraphics` shared component absorbing the 0/1/2+ platform
icon logic from `SecondaryNavigationProjectIcon`
- Refactor `SecondaryNavigationProjectIcon` to delegate to `LeadingGraphics`
- Preserve the old `Breadcrumbs` + `Crumb` API as a compatibility shim so
existing call sites need no changes
- Add stories for `BreadcrumbList` and `LeadingGraphics`
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove the `as` prop from `BreadcrumbList` — always renders as a semantic `<nav aria-label="Breadcrumbs">` since breadcrumbs appear on nearly every page and are navigation by definition - Tighten `leadingGraphic` from `React.ReactNode` to `React.ReactElement<LeadingGraphicsProps>` on all three item components, enforcing use of `LeadingGraphics` at the type level Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace breadcrumbList.stories.tsx with a breadcrumbList.mdx story that documents all item types, leading graphics, pagination, trailing actions, and the container-query overflow behaviour with a live SizingWindow demo. Also tighten component APIs based on review: - BreadcrumbDividerCombo: update display prop type - BreadcrumbItemSelectProjects: remove unused leadingGraphic prop - LeadingGraphics: narrow icon prop to ReactElement, inject size='md' default via cloneElement so callers don't need to specify it explicitly - LeadingGraphics stories: update to reflect new icon prop type Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The BreadcrumbList container query never collapsed: the show/hide toggle
was routed through Flex, whose display resolver defaults every unspecified
breakpoint/axis slot to `flex`. That emitted an always-matching
`@media (min-width: 0px)` rule that shadowed the `@container` query and pinned
parent crumbs visible. Route the toggle through Container instead, which skips
unspecified slots so the container query drives the collapse. Non-link parents
(e.g. select-projects) now collapse below 800px too, matching the spec.
Accessibility fixes for the new items:
- Render the current-page crumb as an <h1> (it is the page's primary heading)
while retaining the prior visual styling.
- Mark it with aria-current="page".
- Hide decorative icons (divider slash, overflow ellipsis, pagination chevrons)
from assistive tech via aria-hidden.
- Give the select-projects trigger a descriptive accessible name
("Selected Project: <value>"), since CompactSelect does not forward a
top-level aria-label to its trigger.
- Wrap the overflow button label in t() for i18n.
Add breadcrumbList.spec.tsx covering the collapse behavior and a11y invariants.
Co-Authored-By: Claude <noreply@anthropic.com>
Replace the index.stories.tsx story with a leadingGraphics.mdx doc that follows the core component conventions: frontmatter (title, description, category, source), an intro, a Usage section with an import block, live Storybook.Demo/Grid examples with copyable code, auto-generated API tabs via the type-loader, and an accessibility section. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Story previewsPreview the stories changed in this PR on the Vercel deployment:
Preview deployment: https://sentry-qby59hwom.sentry.dev |
Contributor
📊 Type Coverage Diff
🔍 2 new type safety issues introduced
Non-null assertions (
This is informational only and does not block the PR. |
Rename the component, its props/variant types, the directory, the @sentry/scraps import path, and the MDX story from the plural LeadingGraphics to the singular LeadingGraphic. Update the breadcrumb items and secondary navigation that consume it. No behavior change. Co-Authored-By: Claude <noreply@anthropic.com>
The legacy Breadcrumbs shim coerced any non-string label to '', silently dropping crumbs whose label is React content (editable page titles/IDs on Discover, Issues, Automations, and Detectors). This blanked out the current crumb on those pages and was the cause of the acceptance-test timeouts. Rewrite the shim as a faithful port of the original component so ReactNode labels render as-is, always rendering as a <nav> landmark. Because it no longer delegates to the string-only typed API, the redundant as="nav" at the two remaining call sites is dropped. Also fix the container-query collapse leaking containerType onto the DOM: emotion's as swap bypasses the wrapper that strips the prop, so move containerType onto a nested plain Container that still establishes the query container at full width. Keep the not-yet-consumed typed API (BreadcrumbList/BreadcrumbListProps/ BreadcrumbItem) exported and tagged @public for a downstream migration PR; keep pure implementation-detail interfaces internal. Drop dead ariaLabel fallbacks now that the pagination prop is required. Add a regression test covering React-node crumb labels. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The earlier legacy shim rerouted the deprecated Breadcrumbs through the redesigned component and always rendered a <nav aria-label>, which hijacked the accessible name of the page <h1> on the ~23 pages that nest breadcrumbs inside the TopBar title heading. Revert that: legacy Breadcrumbs (and the two as="nav" call sites) are restored byte-identical to master, with no call-site migration. Move the redesigned component to its own directory (sentry/components/breadcrumbList) and render it as inline content — no <nav> landmark and no own <h1> — so it nests cleanly inside the TopBar title <h1>, which owns the heading/landmark semantics. The typed API and container-query collapse are unchanged. Migrate the detector details header (views/detectors/components/details/common/ header.tsx) to BreadcrumbList as the first production consumer, which is required for knip:prod to pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Render the current-page crumb as <Text bold> so it reads as the emphasized page title. This restores the medium weight the crumb had while it was a <Heading size="md"> before being switched to inline text. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reworks page breadcrumbs from a positional
crumbsarray intoBreadcrumbList,where each slot is an explicit typed variant (
link,page-title,select-projects) rather than inferred from its position. Each variant owns itsown props, and a new shared
LeadingGraphicprimitive (icon / avatar /project-badge) renders leading glyphs for both breadcrumbs and secondary
navigation. A
Breadcrumbscompatibility shim keeps existingcrumbs={[...]}call sites working.
Responsive collapse
Below the
sm(800px) container width, every parent crumb collapses:linkcrumbs fold into a single
…overflow menu, other parent types simply hide, andthe last crumb always stays visible. This runs on CSS container queries — the
toggle is applied via
Container(notFlex, whosedisplayresolver wouldemit an always-matching
@media (min-width: 0px)rule and defeat the query).Accessibility
<h1>(it is the page's primary heading)with
aria-current=\"page\", while keeping its prior visual styling.aria-hidden.select-projectstrigger gets a descriptive name ("Selected Project:"), since
CompactSelectdoes not forward a top-levelaria-label.Docs
breadcrumbList.mdxandleadingGraphics.mdxdocument both components in thestandard MDX story format (intro, usage, live demos, generated API tabs, a11y).
breadcrumbList.spec.tsxcovers the collapse behavior and the a11y invariants.Known follow-ups (draft)
Two pre-existing issues currently keep CI red and are intentionally left for a
separate change:
Breadcrumbscompatibility shim no longer accepts anasprop, breakingtwo existing call sites (
eventDrawer.tsx,widgetBuilderSlideout.tsx) ontypecheck.
containerTypeleaks a stray DOM attribute from the core layoutContainerprimitive (the CSS still applies), which trips the
console.errorguard inthe legacy
breadcrumbs.spec.tsx.