DataAssetsHeader breadcrumbs: auto-collapse on overflow + render each crumb once#29354
DataAssetsHeader breadcrumbs: auto-collapse on overflow + render each crumb once#29354siddhant1 wants to merge 11 commits into
Conversation
The new DataAssetsHeader (entity page header v2) renders its breadcrumb trail via HeaderBreadcrumb. On entities with deep ancestry the trail could grow wider than its row and overflow instead of collapsing. The core Breadcrumbs component already supports responsive collapse via its `autoCollapse` prop (ResizeObserver-driven: middle crumbs fold into a `…` menu, keeping the first and current crumbs visible, and re-expand when space returns). This exposes that prop through the HeaderBreadcrumb wrapper and turns it on in DataAssetsHeader — no core or MUI changes. - HeaderBreadcrumb: new optional `autoCollapse` prop, passed straight through to the core Breadcrumbs (defaults off, so other consumers are unaffected) - DataAssetsHeader: opt in (its breadcrumb already sits in a min-w-0 flex-1 container, exactly what autoCollapse needs) - HeaderBreadcrumb.test.tsx: unit tests for the passthrough - Container.spec.ts: collapse-aware ancestor read/click helpers (read and navigate via the overflow menu when a crumb is hidden) plus a narrow- viewport test proving collapse engages on overflow Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The v2 DataAssetsHeader appends the current entity itself as the last breadcrumb crumb, assuming the builder output is ancestors-only. That held for ~30 entity types, but the database, database-schema and metric builders (`getBreadcrumbForDatabase`, `getBreadcrumbForDatabaseSchema`, `getBreadcrumbForMetric`) ignored the `includeCurrent` flag their sibling builders honor and always appended the current entity — so its name rendered twice (e.g. "… › Database › Database"). Fix the inconsistency at the source rather than de-duplicating in the header: - The three builders now take `includeCurrent = false` and append the current crumb only when true, matching `getBreadcrumbForDatabaseService` and the table-family builders. - `getEntityBreadcrumbs` forwards `includeCurrent` to the DATABASE / DATABASE_SCHEMA / METRIC cases (it already did for TABLE, DATABASE_SERVICE, DIRECTORY…). - `getDataAssetsHeaderInfo` drops the dead `true` on its DATABASE_SCHEMA call, so its own appended crumb is the single source of the current entity. No other consumer relies on these builders including the current entity: version pages render the name via a separate EntityHeaderTitle, search cards via EntityHeader, and ExploreSearchCard / EntityImportUtils pass `includeCurrent=true` explicitly — all verified against their test suites. Tests: - EntityBreadcrumbPureUtils.test.tsx: default excludes the current entity, `includeCurrent=true` includes it (DATABASE / DATABASE_SCHEMA / METRIC). - DataAssetsHeader.breadcrumb.test.tsx: real-builder regression over every entity type asserting the current entity is never in the trail. - EntityHeaderBreadcrumb.spec.ts: per-entity e2e asserting each crumb renders exactly once (19 entity types). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
siddhant1
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…Container.spec.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
siddhant1
added
the
safe to test
Jest test CoverageUI tests summary
|
🔴 Playwright Results — 9 failure(s), 26 flaky✅ 3599 passed · ❌ 9 failed · 🟡 26 flaky · ⏭️ 168 skipped
Genuine Failures (failed on all attempts)❌
|
…ing-breadcrumbs # Conflicts: # openmetadata-ui/src/main/resources/ui/src/components/common/HeaderBreadcrumb/HeaderBreadcrumb.component.tsx
Code Review 👍 Approved with suggestions 1 resolved / 2 findingsEnables auto-collapse for DataAssetsHeader breadcrumbs and enforces unique crumb rendering for databases, schemas, and metrics. While approved, please address the minor bypass of the getPrioritizedEditPermission pattern in the editStylePermission logic. 💡 Quality: editStylePermission bypasses getPrioritizedEditPermission patternEvery sibling permission in this useMemo (domain/owner/tier/certification) is derived via getPrioritizedEditPermission(permissions, Operation.X), which respects prioritized/override policies, whereas editStylePermission uses the raw permissions.EditAll. If style edits are meant to honor the same prioritization semantics, this will diverge for users whose access comes through prioritized permissions rather than EditAll. Consider using getPrioritizedEditPermission with the appropriate Operation to stay consistent, or confirm EditAll is intentional. ✅ 1 resolved✅ Edge Case: Default breadcrumb change affects shared non-header consumers
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Describe your changes:
No linked issue — UI improvements to the entity-header (v2) breadcrumb.
Two related improvements to the new
DataAssetsHeaderbreadcrumb:1. Auto-collapse on overflow. On entities with deep ancestry the breadcrumb trail shares its row with the follow / version / votes stats and could overflow. The core
Breadcrumbscomponent (@openmetadata/ui-core-components) already supports responsive collapse viaautoCollapse— aResizeObserver-driven loop that folds the middle crumbs into a…menu while keeping the first (root/service) and last (current entity) crumbs visible, and re-expands when space returns. This exposes that prop through theHeaderBreadcrumbwrapper and enables it inDataAssetsHeader— no core or MUI changes.2. Render each crumb exactly once. The header appends the current entity as the last crumb, assuming the builder output is ancestors-only — true for ~30 entity types, but the database / database-schema / metric builders ignored the
includeCurrentflag their siblings honor and always appended the current entity, so its name rendered twice (e.g.… › Database › Database). Fixed at the source: those three builders now honorincludeCurrent(defaultfalse), the dispatcher forwards it, and the header's own appended crumb becomes the single source of the current entity.Type of change:
High-level design:
HeaderBreadcrumbgains an optionalautoCollapseprop (default off → the other ~10 consumers are unaffected), passed straight to the coreBreadcrumbs.DataAssetsHeaderopts in — its breadcrumb already sits in atw:min-w-0 tw:flex-1container, exactly whatautoCollapsemeasures against. Thedata-testid="breadcrumb"stays on the list element, so existing selectors keep working.getBreadcrumbForDatabase/...DatabaseSchema/...MetrictakeincludeCurrent = falseand push the current crumb only whentrue(same shape as the existinggetBreadcrumbForDatabaseService);getEntityBreadcrumbsforwards the flag for the DATABASE / DATABASE_SCHEMA / METRIC cases (it already did for TABLE, DATABASE_SERVICE, DIRECTORY…);getDataAssetsHeaderInfodrops a deadtrueon its DATABASE_SCHEMA call.EntityHeaderTitle(and TABLE version pages already use ancestors-only into the same header), search cards viaEntityHeader/getEntityName, andExploreSearchCard/EntityImportUtilspassincludeCurrent=trueexplicitly. All verified against their test suites.CondensedBreadcrumb(the core already solves overflow and we're removing MUI); a header-level slice/de-dup (a band-aid — fixed the inconsistency at the builder instead).Tests:
Use cases covered
…menu instead of overflowing; opening it reveals + navigates the hidden ancestors; widening re-expands.Unit tests
HeaderBreadcrumb.test.tsx—autoCollapseis forwarded to coreBreadcrumbswhen set, off by default.EntityBreadcrumbPureUtils.test.tsx— DATABASE / DATABASE_SCHEMA / METRIC exclude the current entity by default and include it whenincludeCurrent=true.DataAssetsHeader.breadcrumb.test.tsx— real-builder (un-mocked) regression over every entity type asserting the current entity is never in the trail (36 cases).SearchClassBase/FeedUtils/EntityImportUtils/KnowledgeGraph/ExploreSearchCard) — 122 tests, 0 regressions.Backend / ingestion integration tests
Playwright (UI) tests
Container.spec.ts— the deeply-nested container breadcrumb test is now collapse-aware (reads/navigates ancestors via the…overflow menu), plus a narrow-viewport test proving collapse engages.EntityHeaderBreadcrumb.spec.ts— per-entity regression asserting each breadcrumb crumb renders exactly once; 19 entity types pass against the local stack.Manual testing performed
…, first + current crumbs stay, widening re-expands.UI screen recording / screenshots:
To be attached — breadcrumb collapsing/expanding on resize, and a before/after of the database breadcrumb showing the name once.
Checklist:
🤖 Generated with Claude Code
Greptile Summary
This PR improves the
DataAssetsHeaderbreadcrumb behavior. The main changes are:autoCollapsethrough theHeaderBreadcrumbwrapper.includeCurrent.Confidence Score: 4/5
This is close, but the collapse test should be tightened before merging.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Container.spec.ts
Important Files Changed
autoCollapseoption to the core breadcrumb component.includeCurrentfor database, database schema, and metric breadcrumb generation.Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
Context used (3)