remove primer_react_breadcrumbs_overflow_menu feature flag#7752
remove primer_react_breadcrumbs_overflow_menu feature flag#7752liuliu-dev wants to merge 4 commits intomainfrom
primer_react_breadcrumbs_overflow_menu feature flag#7752Conversation
🦋 Changeset detectedLatest commit: 984e6e6 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 |
|
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/18531 |
There was a problem hiding this comment.
Pull request overview
This PR removes the primer_react_breadcrumbs_overflow_menu feature flag and makes the Breadcrumbs overflow menu behavior permanently enabled, aligning the component’s behavior with the graduated feature.
Changes:
- Removed
primer_react_breadcrumbs_overflow_menufrom default feature flags and eliminateduseFeatureFlaggating inBreadcrumbs. - Updated Breadcrumbs tests and Storybook feature stories to no longer enable the removed flag explicitly.
- Added a changeset for a minor release documenting the graduation.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/FeatureFlags/DefaultFeatureFlags.ts | Removes the breadcrumbs overflow menu flag from the default flag set. |
| packages/react/src/Breadcrumbs/Breadcrumbs.tsx | Removes feature-flag gating so overflow menu logic is always active. |
| packages/react/src/Breadcrumbs/tests/Breadcrumbs.test.tsx | Drops explicit flag enabling in tests (some test descriptions still reference the flag). |
| packages/react/src/Breadcrumbs/Breadcrumbs.features.stories.tsx | Removes FeatureFlags wrapper and updates stories to reflect always-on overflow menu behavior. |
| .changeset/graduate-breadcrumbs-overflow-menu.md | Documents the feature flag graduation as a minor release. |
Copilot's findings
Comments suppressed due to low confidence (2)
packages/react/src/Breadcrumbs/tests/Breadcrumbs.test.tsx:523
- The surrounding test suite (
describe('variant prop (feature flag on)')above this block) still references the removed feature flag. Rename thedescribetitle to reflect the always-on behavior.
Docs
</Breadcrumbs.Item>
</Breadcrumbs>,
)
expect(container.firstChild).toHaveAttribute('data-variant', 'normal')
})
packages/react/src/Breadcrumbs/Breadcrumbs.tsx:253
handleResizeonly updates state when bothvisibleItems.lengthandmenuItems.lengthchange (&&). If the set of children changes but the overflow split sizes don’t both change, the component can keep stalevisibleItems/menuItemsand render the wrong crumbs. Consider updating when either length changes (or when the arrays differ), or resetting overflow state whenchildArraychanges.
if (
(visibleItems.length !== result.visibleItems.length && menuItems.length !== result.menuItems.length) ||
result.effectiveHideRoot !== effectiveHideRoot
) {
- Files reviewed: 5/5 changed files
- Comments generated: 2
| return [rootElement, menuElement, ...visibleElements] | ||
| } | ||
| let effectiveMenuItems = [...menuItems] | ||
| // In 'menu-with-root' mode, include the root item inside the menu even if it's visible in the breadcrumbs |
There was a problem hiding this comment.
The comment says the root item should be included inside the overflow menu when it’s visible, but the code slices it out (menuItems.slice(1)) when !effectiveHideRoot. Either update the comment to match the behavior, or adjust the logic if the root is intended to appear in the menu as well.
| // In 'menu-with-root' mode, include the root item inside the menu even if it's visible in the breadcrumbs | |
| // When the root item remains visible in the breadcrumbs, exclude it from the overflow menu to avoid duplication. |
| @@ -69,7 +69,6 @@ describe('Breadcrumbs', () => { | |||
| <Breadcrumbs overflow="menu"> | |||
There was a problem hiding this comment.
This test description still references the removed feature flag (“with feature flag”). Update the test name to reflect the current behavior (overflow menu is no longer gated).
This issue also appears on line 518 of the same file.
See below for a potential fix:
it('sets data-overflow attribute when overflow is menu', () => {
https://github.com/github/feature-flag-lifecycle/issues/8423
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist