feat(ui): add optional renderPageHeader override to Domain & Data Product list pages#29720
feat(ui): add optional renderPageHeader override to Domain & Data Product list pages#29720siddhant1 wants to merge 1 commit into
Conversation
…duct list pages Add an optional renderPageHeader render prop to DomainListPage and DataProductListPage. When provided it replaces the default page header and the breadcrumb; when omitted the pages render their existing header unchanged. Props live in colocated *.interface.ts files. Generic, backward-compatible extension point with no behavior change for existing callers.
❌ 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! |
siddhant1
added
UI
Code Review ✅ ApprovedAdds an optional renderPageHeader prop to Domain and Data Product list pages, allowing for custom header injection while maintaining existing default functionality. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
There was a problem hiding this comment.
Pull request overview
Adds an optional renderPageHeader render prop to the Domain and Data Product listing pages so callers can fully override the header area (including breadcrumb) while preserving the existing default UI when the prop is omitted.
Changes:
DomainListPage/DataProductListPageaccept optionalrenderPageHeaderand render it in place of the default breadcrumb + header when provided.- Introduces colocated
*.interface.tsprop type files for both list pages.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/DomainListing/DomainListPage.tsx | Adds renderPageHeader override path and suppresses breadcrumb when used. |
| openmetadata-ui/src/main/resources/ui/src/components/DomainListing/DomainListPage.interface.ts | Defines DomainListPageProps including optional renderPageHeader. |
| openmetadata-ui/src/main/resources/ui/src/components/DataProduct/DataProductListPage.tsx | Adds renderPageHeader override path and suppresses breadcrumb when used. |
| openmetadata-ui/src/main/resources/ui/src/components/DataProduct/DataProductListPage.interface.ts | Defines DataProductListPageProps including optional renderPageHeader. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export interface DataProductListPageProps { | ||
| pageTitle: string; | ||
| renderPageHeader?: (props: { | ||
| onAddClick: () => void; | ||
| createPermission: boolean; | ||
| count: number; | ||
| }) => ReactNode; | ||
| } |
There was a problem hiding this comment.
renderPageHeader is identical in both DataProductListPageProps and DomainListPageProps. Extracting it to a shared named type avoids silent drift if one signature changes without the other.
| export interface DataProductListPageProps { | |
| pageTitle: string; | |
| renderPageHeader?: (props: { | |
| onAddClick: () => void; | |
| createPermission: boolean; | |
| count: number; | |
| }) => ReactNode; | |
| } | |
| export interface PageHeaderRenderProps { | |
| onAddClick: () => void; | |
| createPermission: boolean; | |
| count: number; | |
| } | |
| export interface DataProductListPageProps { | |
| pageTitle: string; | |
| renderPageHeader?: (props: PageHeaderRenderProps) => ReactNode; | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export interface DataProductListPageProps { | ||
| pageTitle: string; | ||
| renderPageHeader?: (props: { | ||
| onAddClick: () => void; | ||
| createPermission: boolean; | ||
| count: number; | ||
| }) => ReactNode; | ||
| } |
There was a problem hiding this comment.
pageTitle required but unused in component body
pageTitle is declared as required here but the component only destructures renderPageHeader. The prop is intentionally consumed by withPageLayout (which spreads all props into PageLayoutV1), but this is non-obvious to future readers. A short JSDoc comment — e.g. /** Consumed by withPageLayout for the document title. */ — above the field would prevent well-meaning refactors that accidentally drop it.
Jest test CoverageUI tests summary
|
|
🔴 Playwright Results — 2 failure(s), 23 flaky✅ 4478 passed · ❌ 2 failed · 🟡 23 flaky · ⏭️ 38 skipped
Genuine Failures (failed on all attempts)❌
|
What
Adds an optional
renderPageHeaderrender prop toDomainListPageandDataProductListPage.Prop types live in colocated
DomainListPage.interface.ts/DataProductListPage.interface.ts, following the existing*.interface.tsconvention.Why
A generic, backward-compatible extension point that lets a downstream app inject a custom list-page header (e.g. a branded header) while reusing the page's list body, drawer, and permission wiring. There is no behavior change for existing callers — the new prop is optional and defaults to today's rendering.
Changes
DomainListPage/DataProductListPage: accept optionalrenderPageHeader({ onAddClick, createPermission, count }); render it in place of the default header + breadcrumb when present.*.interface.tsfiles for the two props types.Testing
renderPageHeaderprovided → returned node renders, breadcrumb suppressed.Greptile Summary
This PR adds an optional
renderPageHeaderrender prop to bothDomainListPageandDataProductListPage. When the prop is omitted the pages behave exactly as before; when it is provided, the default breadcrumb and page header are replaced by the caller's node while the list body, drawer, and permission wiring remain unchanged.DomainListPage.interface.ts/DataProductListPage.interface.tsfiles export the props types, following the existing*.interface.tsconvention.HeaderBreadcrumbwith!renderPageHeaderand use a ternary to select between the custom node and the existingpageHeader— the two render paths are logically symmetric and backward-compatible.pageTitleis kept as a required field in both interfaces becausewithPageLayoutneeds it to satisfy itsP extends { pageTitle: string }constraint and populatePageLayoutV1; existing router call-sites already pass it.Confidence Score: 4/5
Safe to merge — the change is purely additive, existing render paths are untouched, and the router call-sites already satisfy the required pageTitle prop.
The logic is correct and backward-compatible. The only observations are stylistic: the render-prop callback shape is copy-pasted verbatim into both interface files with no shared type, and the pageTitle field's purpose (satisfying withPageLayout) is invisible from the interface alone. Neither affects runtime behavior.
Both *.interface.ts files warrant a quick look for the duplicated inline callback type and the undocumented pageTitle field.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Caller["Router / Caller"] Caller -->|"pageTitle + renderPageHeader?"| WrappedPage["withPageLayout(DomainListPage / DataProductListPage)"] WrappedPage -->|"pageTitle"| PageLayoutV1["PageLayoutV1 (document title)"] WrappedPage -->|"all props"| InnerPage["Inner Page Component"] InnerPage --> CheckHeader{renderPageHeader\nprovided?} CheckHeader -->|"Yes"| NoBC["Breadcrumb suppressed"] CheckHeader -->|"Yes"| CustomHeader["renderPageHeader({ onAddClick, createPermission, count })"] CheckHeader -->|"No"| ShowBC["HeaderBreadcrumb rendered"] CheckHeader -->|"No"| DefaultHeader["Default usePageHeader node rendered"] NoBC & CustomHeader --> Body["Card body: filters, list/card/tree, pagination"] ShowBC & DefaultHeader --> Body%%{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 Caller["Router / Caller"] Caller -->|"pageTitle + renderPageHeader?"| WrappedPage["withPageLayout(DomainListPage / DataProductListPage)"] WrappedPage -->|"pageTitle"| PageLayoutV1["PageLayoutV1 (document title)"] WrappedPage -->|"all props"| InnerPage["Inner Page Component"] InnerPage --> CheckHeader{renderPageHeader\nprovided?} CheckHeader -->|"Yes"| NoBC["Breadcrumb suppressed"] CheckHeader -->|"Yes"| CustomHeader["renderPageHeader({ onAddClick, createPermission, count })"] CheckHeader -->|"No"| ShowBC["HeaderBreadcrumb rendered"] CheckHeader -->|"No"| DefaultHeader["Default usePageHeader node rendered"] NoBC & CustomHeader --> Body["Card body: filters, list/card/tree, pagination"] ShowBC & DefaultHeader --> BodyReviews (1): Last reviewed commit: "feat(ui): add optional renderPageHeader ..." | Re-trigger Greptile