Fixed context center breadcrumb home icon#29567
Conversation
Rohit0301
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
🟡 Playwright Results — all passed (51 flaky)✅ 4435 passed · ❌ 0 failed · 🟡 51 flaky · ⏭️ 37 skipped
🟡 51 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Code Review ✅ Approved 1 resolved / 1 findingsCentralizes root breadcrumb construction and adds conditional home icon support for embedded mode. All findings related to test coverage and logic gaps have been resolved. ✅ 1 resolved✅ Quality: resolvedBreadcrumbs silently drops first item via slice(1)
OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…nter/ArticleDetailHeader/ArticleDetailHeader.test.tsx Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Jest test CoverageUI tests summary
|
|
Code Review 👍 Approved with suggestions 1 resolved / 2 findingsCentralizes Context Center breadcrumb construction into the base class to remove redundant definitions across pages. Please address the missing 💡 Quality: Hardcoded download filename string literal
✅ 1 resolved✅ Quality: resolvedBreadcrumbs silently drops first item via slice(1)
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
3 participants
Describe your changes:
Fixes #
I worked on ... because ...
Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
Unit tests
Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Greptile Summary
This PR centralizes the Context Center breadcrumb home icon/root item by moving its construction into
ContextCenterClassBase.getContextCenterRootBreadcrumb(t)and auto-prepending it inContextCenterHeader, so individual pages no longer need to pass it explicitly. It also replaces the inlinestylecard prop with agetHeaderCardClassName()CSS class approach and conditionally shows theshowHomebreadcrumb icon only when not in embedded mode.ContextCenterClassBase: removesgetCardStyle()andgetBreadcrumbClassName(), addsgetHeaderCardClassName()andgetContextCenterRootBreadcrumb(t), and introduces theContextCenterBreadcrumbIteminterface.ArticleDetailHeader,ArticleVersionHeader,ContextCenterHeader): consume the new class-base methods;showHomeis now!isEmbeddedMode().breadcrumbsprop.Confidence Score: 5/5
Safe to merge — the refactoring is self-contained, all five affected page components are consistently updated, and the one test-file mock returns the wrong type but does not break any test at runtime.
All component and page changes are consistent and correctly remove the now-centralized root breadcrumb item. The only irregularity is in the test file where
getHeaderCardClassNamemock returns{}instead of'', which is harmless at runtime becauseclassNames({})produces an empty string, identical in effect.ArticleDetailHeader.test.tsx — the
getHeaderCardClassNamemock return value is an object instead of a string.Important Files Changed
ContextCenterBreadcrumbIteminterface, replacesgetCardStyle()/getBreadcrumbClassName()withgetHeaderCardClassName(), and introducesgetContextCenterRootBreadcrumb(t)to centralize the root breadcrumb construction.getContextCenterRootBreadcrumb(t)for the first breadcrumb item, conditionally shows the home icon based onisEmbeddedMode(), and switches from inlinestyletoclassNamefor card styling.isEmbeddedModeandgetContextCenterRootBreadcrumb; however,getHeaderCardClassNamemock incorrectly returns{}(object) instead of''(string).getContextCenterRootBreadcrumb,isEmbeddedMode, andgetHeaderCardClassName.resolvedBreadcrumbs, conditionsshowHomeon!isEmbedded, and replacesstyleprop withclassNames-based className approach.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["Page Component\n(Archive / Articles / Dashboard / Documents / Memories)"] B["breadcrumbs prop\n[{ label: 'Archive' }, ...]"] C["ContextCenterHeader"] D["getContextCenterRootBreadcrumb(t)\n→ { label: 'Context Center', href: '/context-center' }"] E["resolvedBreadcrumbs\n= [root, ...breadcrumbs]"] F["isEmbeddedMode()"] G["showHome = !isEmbedded"] H["HeaderBreadcrumb\nitems=resolvedBreadcrumbs, showHome=!isEmbedded"] A -->|"passes"| B B --> C C --> D D --> E C --> F F --> G E --> H G --> H%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A["Page Component\n(Archive / Articles / Dashboard / Documents / Memories)"] B["breadcrumbs prop\n[{ label: 'Archive' }, ...]"] C["ContextCenterHeader"] D["getContextCenterRootBreadcrumb(t)\n→ { label: 'Context Center', href: '/context-center' }"] E["resolvedBreadcrumbs\n= [root, ...breadcrumbs]"] F["isEmbeddedMode()"] G["showHome = !isEmbedded"] H["HeaderBreadcrumb\nitems=resolvedBreadcrumbs, showHome=!isEmbedded"] A -->|"passes"| B B --> C C --> D D --> E C --> F F --> G E --> H G --> HComments Outside Diff (2)
openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ArticleDetailHeader/ArticleDetailHeader.test.tsx, line 55-68 (link)contextCenterClassBasemethodsThe mock object for
contextCenterClassBaseno longer includesisEmbeddedModeorgetContextCenterRootBreadcrumb, both of which are now called at the top of the component. When any test rendersArticleDetailHeader, JavaScript will findundefinedfor those properties and immediately throwTypeError: contextCenterClassBase.isEmbeddedMode is not a functionbefore the component can mount, causing every test in this suite to fail.openmetadata-ui/src/main/resources/ui/src/components/ContextCenter/ArticleDetailHeader/ArticleDetailHeader.test.tsx, line 55-68 (link)contextCenterClassBasemethods cause every test to failThe component now calls
contextCenterClassBase.isEmbeddedMode()unconditionally at the top of the component body (beforeuseMemo) andcontextCenterClassBase.getContextCenterRootBreadcrumb(t)insideuseMemo. Neither is present in the mock object, so every test throwsTypeError: contextCenterClassBase.isEmbeddedMode is not a functionbefore the component can mount. Add the two missing mock entries:isEmbeddedMode: jest.fn(() => false)andgetContextCenterRootBreadcrumb: jest.fn(() => ({ label: 'label.context-center', href: '/context-center' })).Reviews (13): Last reviewed commit: "Merge branch 'main' into context-center-..." | Re-trigger Greptile