Repository navigation
feat: Global breadcrumbs feature flag - #5113
georgylobko wants to merge 3 commits into
Conversation
1437f3f to
acc0c77
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5113 +/- ##
=======================================
Coverage 97.70% 97.70%
=======================================
Files 990 990
Lines 31818 31821 +3
Branches 11752 11753 +1
=======================================
+ Hits 31089 31092 +3
Misses 722 722
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Renaming the cross-version widget state key without a compatibility fallback can restore duplicate toolbar breadcrumbs in mixed-version deployments.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds a global feature flag to hide AppLayout toolbar breadcrumbs from the first render, preventing flicker before an external consumer registers.
Changes:
- Adds and propagates
appLayoutHideToolbarBreadcrumbs. - Renames the widgetized visibility state.
- Adds flag behavior tests and updates contract snapshots.
| File | Description |
|---|---|
src/app-layout/visual-refresh-toolbar/widget-areas/before-main-slot.tsx |
Passes breadcrumb visibility state to the toolbar. |
src/app-layout/visual-refresh-toolbar/toolbar/index.tsx |
Adopts the renamed toolbar property. |
src/app-layout/visual-refresh-toolbar/state/use-app-layout.tsx |
Combines consumer registration with the global flag. |
src/app-layout/visual-refresh-toolbar/skeleton/toolbar-container.tsx |
Hides breadcrumbs based on the new state. |
src/app-layout/visual-refresh-toolbar/interfaces.ts |
Updates the widgetized state interface. |
src/app-layout/utils/feature-flags.ts |
Reads the new global flag. |
src/app-layout/__tests__/runtime-breadcrumbs-widgetized.test.tsx |
Tests flagged breadcrumb behavior. |
src/app-layout/__tests__/__snapshots__/widget-contract-split-panel-old.test.tsx.snap |
Updates split-panel widget contracts. |
src/app-layout/__tests__/__snapshots__/widget-contract-old.test.tsx.snap |
Updates widget contract snapshots. |
pages/app/index.tsx |
Exposes the flag through development-page query parameters. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| widgetizedState: { | ||
| ...appLayoutInternals, | ||
| breadcrumbsExternallyOwned: hasBreadcrumbsConsumer, | ||
| hideBreadcrumbsInToolbar: hasBreadcrumbsConsumer || isAppLayoutHideToolbarBreadcrumbsEnabled(), |
There was a problem hiding this comment.
It doesn’t cause a break, since use-app-layout loads as a widget alongside the other widget parts. There are no breaking changes here
| bottomDrawerReportedSize, | ||
| featureNotificationsProps, | ||
| breadcrumbsExternallyOwned, | ||
| hideBreadcrumbsInToolbar, |
There was a problem hiding this comment.
Same as in the previous comment
Description
Adds a feature flag that hides breadcrumbs in the AppLayout toolbar from the first render. This PR made it possible for an external consumer to take over breadcrumbs, and AppLayout then hides its own toolbar breadcrumbs. The consumer registers through an async event, though, so the breadcrumbs only get hidden after the first render, which causes a visible flicker. With this flag on, the toolbar breadcrumbs never render, so the flicker goes away.
Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.