Untitled UI migration for platform/discovery#26312
Conversation
Rohit0301
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
|
||
| const lineageCall = | ||
| mockLineage.mock.calls[mockLineage.mock.calls.length - 1]; | ||
| const lineageCall = mockLineage.mock.calls.at(-1); |
There was a problem hiding this comment.
💡 Edge Case: PlatformLineage test uses .at(-1) which can return undefined
In PlatformLineage.test.tsx, mockLineage.mock.calls.at(-1) replaced mockLineage.mock.calls[mockLineage.mock.calls.length - 1]. While functionally equivalent when calls exist, .at(-1) returns undefined when the array is empty, and the next line (lineageCall[0].platformHeader) would throw a TypeError. The old code had the same issue, but since this is a refactor opportunity, consider adding a guard or assertion.
Suggested fix:
const lineageCall = mockLineage.mock.calls.at(-1);
expect(lineageCall).toBeDefined();
const platformHeader = lineageCall![0].platformHeader;
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
| mb: theme.spacing(3), | ||
| }}> | ||
| > | ||
| <Typography as="p" className="tw:font-bold tw:text-gray-400"> |
There was a problem hiding this comment.
💡 Quality: Lineage header label style mismatch: tw:text-gray-400 appears too light
In LineageSection.tsx line 155, the 'Lineage' section header uses tw:text-gray-400 which is typically a very light gray (#9CA3AF). The old code used fontWeight: theme.typography.h1.fontWeight with default text color, suggesting this should be a dark/prominent heading. tw:font-bold tw:text-gray-400 produces bold but light text, which seems like an unintentional visual regression. Consider using tw:text-gray-700 or tw:text-gray-900 for readable header text.
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
🔍 CI failure analysis for 02f76a9: One flaky Playwright E2E test failed due to an Airflow API internal server error during pipeline redeployment; 291 out of 313 ingestion tests passed successfully, indicating infrastructure instability rather than PR-related failures.OverviewAnalysis of 20 CI logs from the platform-discovery-migration PR reveals a single infrastructure-related failure. The UI migration PR itself built successfully with no compilation or test failures directly attributable to code changes. The 1 test failure is an intermittent Playwright E2E test that failed due to upstream Airflow service issues, not application logic problems. FailuresAirflow Pipeline Redeployment Flaky Test (confidence: medium)
Summary
Code Review 👍 Approved with suggestions 13 resolved / 15 findingsUntitled UI migration for platform/discovery with comprehensive fixes addressing 13 prior issues including CSS class prefixes, non-null assertions, and component styling inconsistencies. Two minor findings remain: PlatformLineage test's .at(-1) can return undefined, and LineageSection header label style appears too light. 💡 Edge Case: PlatformLineage test uses .at(-1) which can return undefined📄 openmetadata-ui/src/main/resources/ui/src/pages/PlatformLineage/PlatformLineage.test.tsx:452 📄 openmetadata-ui/src/main/resources/ui/src/pages/PlatformLineage/PlatformLineage.test.tsx:486 📄 openmetadata-ui/src/main/resources/ui/src/pages/PlatformLineage/PlatformLineage.test.tsx:558 📄 openmetadata-ui/src/main/resources/ui/src/pages/PlatformLineage/PlatformLineage.test.tsx:813 In PlatformLineage.test.tsx, Suggested fix💡 Quality: Lineage header label style mismatch: tw:text-gray-400 appears too light📄 openmetadata-ui/src/main/resources/ui/src/components/common/LineageSection/LineageSection.tsx:155 In LineageSection.tsx line 155, the 'Lineage' section header uses ✅ 13 resolved✅ Bug: Drawer title section loses transparent background
✅ Bug: Typo
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|
3 participants
Describe your changes:
Migrated the below components:-
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
platform/discoverycomponents from Untitled UI to standard HTML/CSS, replacingBoxcomponents withdivelementsDataProductNode,PortsLineageView,EntitySummaryPanel,EntityTitleSection,LineageSectioncomponentsuseTranslation()hook to components for i18n supportEntitySummaryPanel.test.tsxand related test files to match migrated componentsThis will update automatically on new commits.