Breadcrumbs : Add overflow menu for responsive behavior #6664

pksjce · 2025-08-25T11:48:50Z

Closes #https://github.com/github/primer/issues/5122

Changelog

Breadcrumbs now show an overflow menu with the following features

If no of items in the breadcrumbs is >5 then we show the menu
If the screen width can't accomodate the items then we show the menu
if hideRoot is false then we show the root/overflow menu/leaf

https://www.loom.com/share/8902798416b741c9959fc7bb501185a5

New

Changed

Removed

Rollout strategy

Patch release
Minor release
Major release; if selected, include a written rollout or migration plan
None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

changeset-bot · 2025-08-25T11:48:55Z

🦋 Changeset detected

Latest commit: a24d75a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@primer/react	Patch
@primer/styled-react	Patch

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

github-actions · 2025-08-25T11:49:21Z

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

github-actions · 2025-08-25T11:52:59Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	90.78 KB (+0.84% 🔺)
packages/react/dist/browser.umd.js	90.98 KB (+0.84% 🔺)

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

strackoverflow

This is looking great!

One edge case issue I uncovered (not necessarily blocking), is that when I have the root item shown and resize the window to a narrower viewport, at certain widths, the component seems to struggle with the root item getting hidden (which may be triggering another resize event), creating an infinite loop of hiding and showing the root crumb:

breadcrumbs.mov

francinelucca · 2025-08-25T18:24:43Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+          />
+        </ActionMenu.Anchor>
+        <ActionMenu.Overlay width="auto">
+          <ActionList role="menu">


does this have the a11y enhancements in? if not, is the plan to follow up this PR with those enhancements? if so:

I think it makes sense for this PR not to close https://github.com/github/primer/issues/5122

would it be a breaking change to get this in and then switch the underlying html structure? 🤔

I created subissues on that issue to tackle the ally issues!

yeah I just mean feels weird to close the parent issue if there's still open subissues

Yes yes, I won't be closing the parent issue. It's just an old description. I just want it linked to the issue

francinelucca · 2025-08-25T18:26:48Z

.changeset/good-cougars-hug.md

+"@primer/react": patch
+---
+
+Spike for breadcrumbs overflow


I think this needs to be updated as it'll show up in the changelog

🤔 I thought i did. I think its fine now. I think something with my cherry-pick messed it

Now there's two 😅 , can you delete this one?

francinelucca · 2025-08-25T18:27:15Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+  // if (typeof window !== 'undefined') {
+  //   effectiveOverflow = overflow
+  // }


francinelucca · 2025-08-25T18:28:34Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+    }
+    const menuElement = (
+      <li className={classes.BreadcrumbsItem} key="breadcrumbs-menu">
+        <BreadcrumbsMenuItem


from https://github.com/github/primer/issues/5122

Next Steps
Merge Breadcrumbs as currently implemented under a feature flag

But I don't see a FF here 👀

I know! I debated it but I decided since its only addition of a prop and only that triggers the new feature, we don't need it. WDYT?

I think the good thing about a FF is we can get away with changing more things within it to refine stuff, but if you don't foresee making any major changes (outside the pre-release window), then a prop is fine 👍

I'm still undecided. There is definite change in the html structure and css to the original. I want to have a look at the review-lab here and decide

joshblack

Looking great! Just left some comments/questions/ideas.

One odd thing I noticed when working with the component is that when I saved the breadcrumb file, the layout would break 🤔 Curious if you ran into that too

Screen.Recording.2025-08-25.at.4.26.56.PM.mov

joshblack · 2025-08-25T15:17:47Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+      setChildArrayWidths(widths)
+      setRootItemWidth(listElementArray[0].offsetWidth)
+    }
+  }, [childArray.length])


I don't think this will synchronize correctly if it depends on the length of the items instead of the items themselves changing.

Put another way, if the items are updated to be different items but of the same length then this code won't re-run and set up the state values we have for item and root width.

joshblack · 2025-08-25T21:19:19Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+      // Show: [root breadcrumb, overflow menu, leaf breadcrumb]
+      return [rootElement, menuElement, ...visibleElements]
+    }
+  }, [overflow, menuItems, effectiveHideRoot, visibleItems, rootItem, children])


If I'm understanding right, I think on every resize this value will be memoized is that right? Even if the items aren't changing. Would it make sense in handleResize to compare the two lists (visible items, menu items) to see if they have changed before updating?

I'm also curious if, with React compiler, we need to use memo for this 🤔 Curious what you've run into for that 👀

joshblack · 2025-08-25T21:21:48Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+const BreadcrumbsMenuItem = React.forwardRef<HTMLButtonElement, BreadcrumbsMenuItemProps>(
+  ({items, 'aria-label': ariaLabel, ...rest}, ref) => {
+    return (
+      <ActionMenu>


I think my personal preference would be to have this use the disclosure pattern before getting this in since using a menu for navigation is not preferred.

I assume with this work that if we went down that route we would have to do more from scratch since that's not supported in ActionMenu at the moment?

Yes, We need to use a pattern involving Details. I tried it once on Friday and it was not straightforward. Will give it another go, now that I have a clean slate

joshblack · 2025-08-25T21:25:30Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+    overflow?: 'wrap' | 'menu'
+    hideRoot?: boolean


Definitely not sure what the right names would be, but what would you think of something like:

Suggested change

overflow?: 'wrap' | 'menu'

hideRoot?: boolean

overflow?: 'wrap' | 'hide' | 'hide-root'

So that we wouldn't have the case of hideRoot only making sense with overflow="menu". It would also avoid the confusion here with it not being a menu since this would be a popup with a list of links versus role="menu"

Oh this may be an improvement, but do you think it makes the api more convoluted? menu is what it practically looks like. hide and hide-root as values dont semantically make sense in this context

Am I correct in saying that hideRoot would only be relevant with overflow: menu? If yes, how do you feel about overflow: 'menu' | 'menu-with-hidden-root'? A little long but keeping related options together?

joshblack · 2025-08-25T21:31:30Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+        setMenuButtonWidth(measuredWidth)
+      }
+    }
+  }, [menuItems.length]) // Re-measure when menu button appears/disappears


One idea for measuring the menu button could be to measure it in a ref callback, curious if that would make sense or not 🤔 Would avoid the need for us to have an effect for it. Hopefully this doesn't violate the rules of hooks too 😅 lol

joshblack · 2025-08-25T21:34:03Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+          const itemToHide = currentVisibleItems.slice(0)[0]
+          const itemToHideWidth = currentVisibleItemWidths.slice(0)[0]


What's the call to .slice(0)[0] for here? Is it to do a deep copy on the item in the array or or something else?

joshblack · 2025-08-25T21:35:05Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+          currentVisibleItems = [...currentVisibleItems.slice(1)]
+          currentVisibleItemWidths = [...currentVisibleItemWidths.slice(1)]


One idea, I think .slice(1) would create a new array here so we could avoid spreading it into a new array

Suggested change

currentVisibleItems = [...currentVisibleItems.slice(1)]

currentVisibleItemWidths = [...currentVisibleItemWidths.slice(1)]

currentVisibleItems = currentVisibleItems.slice(1)

currentVisibleItemWidths = currentVisibleItemWidths.slice(1)

joshblack · 2025-08-25T21:36:36Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+        visibleItems: [...currentVisibleItems],
+        menuItems: [...currentMenuItems],


One idea with the spread:

Suggested change

visibleItems: [...currentVisibleItems],

menuItems: [...currentMenuItems],

visibleItems: currentVisibleItems,

menuItems: currentMenuItems,

Let me know if these spread suggestions aren't applicable here btw!

joshblack · 2025-08-25T21:39:07Z

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

+        <BreadcrumbsMenuItem
+          ref={menuButtonRef}
+          items={effectiveMenuItems}
+          aria-label={`${effectiveMenuItems.length} more breadcrumb items`}


This could be a good use for the … character specifically on toggle element (got the idea from spectrum: https://react-spectrum.adobe.com/react-spectrum/Breadcrumbs.html) and that might mean we don't need a tooltip on this

I remember Katie specifically mentioned that its good to use the iconbutton for the toggle element. Is that what you mean here? Also the tooltip is the same as they used in context-region

pksjce added 22 commits August 25, 2025 21:46

Spike for breadcrumbs overflow

a7208d4

Add changeset for breadcrumbs overflow

54c9148

Add review comments and change behavior

dc5dac1

Fix up some issues.

7df321a

check for zero children

611d703

Fix bug with hideRoot

1a84b8a

Add test cases

c567b15

Add docs

35c0971

Breadcrumbs can have more stories

901ba0c

Add features stories for breadcrumbs

f8fd857

Add wrapped breadcrumbs in story

f415276

Fix for ssr and child key

68eba14

Add IconButton

f4bba96

Fix for SSR

b55755c

Fix for SSR

9477af7

Final changes for SSR

df9aa0f

Rework calculations

fcd8a8e

Tests are passing

0a09e45

Small fixes to menu button

3b9891a

Fix styling issues with button

273a055

Fix focus states

08637ae

Fix focus states

17a3107

Copilot AI review requested due to automatic review settings August 25, 2025 11:48

pksjce requested a review from a team as a code owner August 25, 2025 11:48

pksjce requested a review from francinelucca August 25, 2025 11:48

github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Aug 25, 2025

This comment was marked as spam.

Sign in to view

Make sure old behavior works

ed838f3

pksjce requested a review from joshblack August 25, 2025 11:58

github-actions bot temporarily deployed to storybook-preview-6664 August 25, 2025 11:58 Inactive

pksjce deployed to github-pages August 25, 2025 12:07 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-6664 August 25, 2025 12:08 Inactive

Fix tests and lint

1870eff

pksjce deployed to github-pages August 25, 2025 13:09 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-6664 August 25, 2025 13:10 Inactive

Copilot AI reviewed Aug 25, 2025

View reviewed changes

strackoverflow approved these changes Aug 25, 2025

View reviewed changes

francinelucca reviewed Aug 25, 2025

View reviewed changes

joshblack reviewed Aug 25, 2025

View reviewed changes

pksjce added 2 commits August 26, 2025 09:11

Create eighty-queens-tap.md

0eaf106

Merge branch 'main' into pk/breadcrumbs-with-overflow-menu

a24d75a

github-actions bot temporarily deployed to storybook-preview-6664 August 25, 2025 23:20 Inactive

pksjce deployed to github-pages August 25, 2025 23:29 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-6664 August 25, 2025 23:29 View deployment

	overflow?: 'wrap' \| 'menu'
	hideRoot?: boolean
	overflow?: 'wrap' \| 'hide' \| 'hide-root'

		const itemToHide = currentVisibleItems.slice(0)[0]
		const itemToHideWidth = currentVisibleItemWidths.slice(0)[0]

		currentVisibleItems = [...currentVisibleItems.slice(1)]
		currentVisibleItemWidths = [...currentVisibleItemWidths.slice(1)]

		visibleItems: [...currentVisibleItems],
		menuItems: [...currentMenuItems],

Breadcrumbs : Add overflow menu for responsive behavior #6664

Are you sure you want to change the base?

Breadcrumbs : Add overflow menu for responsive behavior #6664

Conversation

pksjce commented Aug 25, 2025

Changelog

New

Changed

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-bot bot commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Aug 25, 2025

Uh oh!

This comment was marked as spam.

Uh oh!

github-actions bot commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

strackoverflow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshblack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siddharthkp Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

changeset-bot bot commented Aug 25, 2025 •

edited

Loading

github-actions bot commented Aug 25, 2025 •

edited

Loading

siddharthkp Aug 26, 2025 •

edited

Loading