Breadcrumbs: Show only current page on narrow viewports with overflow menu#7754
Breadcrumbs: Show only current page on narrow viewports with overflow menu#7754liuliu-dev wants to merge 5 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 29ad55e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Uh oh! @liuliu-dev, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:
Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
|
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/18622 |
There was a problem hiding this comment.
Pull request overview
Updates the Breadcrumbs overflow behavior to better support small-screen layouts by collapsing to fewer visible items when using menu overflow, and adds VRT coverage for the new/expected responsive rendering.
Changes:
- Adjusted overflow calculation to reduce visible breadcrumb items at the
<544pxbreakpoint foroverflow="menu". - Added Playwright VRT cases for overflow-menu stories at narrow and wide viewport sizes.
- Added new/updated Playwright snapshot baselines and a patch changeset entry.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/Breadcrumbs/Breadcrumbs.tsx | Updates overflow/min-visible-items logic with a narrow breakpoint condition. |
| e2e/components/Breadcrumbs.test.ts | Adds VRT coverage for overflow menu behavior at narrow/wide viewports. |
| .playwright/snapshots/components/Breadcrumbs.test.ts-snapshots/Breadcrumbs-OverflowMenuWithRoot-narrow-linux.png | Adds new VRT baseline for overflow menu with root at narrow viewport. |
| .playwright/snapshots/components/Breadcrumbs.test.ts-snapshots/Breadcrumbs-OverflowMenu-wide-linux.png | Adds new VRT baseline for overflow menu at wide viewport. |
| .playwright/snapshots/components/Breadcrumbs.test.ts-snapshots/Breadcrumbs-OverflowMenu-narrow-linux.png | Adds new VRT baseline for overflow menu at narrow viewport. |
| .changeset/fast-pugs-camp.md | Patch changeset documenting the new narrow-viewport overflow-menu behavior. |
Copilot's findings
- Files reviewed: 3/6 changed files
- Comments generated: 2
| const NARROW_BREAKPOINT = 544 | ||
| const isNarrow = availableWidth < NARROW_BREAKPOINT | ||
| const MIN_VISIBLE_ITEMS = isNarrow && eHideRoot ? 1 : !eHideRoot ? 3 : 4 |
| const NARROW_BREAKPOINT = 544 | ||
| const isNarrow = availableWidth < NARROW_BREAKPOINT |
francinelucca
left a comment
There was a problem hiding this comment.
approving with non-blocking comments!
| const MIN_VISIBLE_ITEMS = !eHideRoot ? 3 : 4 | ||
| const NARROW_BREAKPOINT = 544 | ||
| const isNarrow = availableWidth < NARROW_BREAKPOINT | ||
| const MIN_VISIBLE_ITEMS = isNarrow && eHideRoot ? 1 : !eHideRoot ? 3 : 4 |
There was a problem hiding this comment.
nit: I feel like these double ternaries are hard to read. I'd split this into multiple lines or a function
| '@primer/react': patch | ||
| --- | ||
|
|
||
| Breadcrumbs: On narrow viewports, only show the current page breadcrumb and the overflow menu when `overflow="menu"` is set. |
There was a problem hiding this comment.
I wonder if this should be the default 🤔
Closes https://github.com/github/primer/issues/6371
Changelog
On narrow container width (<544px), when
overflow="menu"is set, the Breadcrumbs component now collapses all items except the current page (last breadcrumb) into the overflow menu.Rollout strategy
Testing & Reviewing
Merge checklist