refactor(ui): migrate utils/* Typography to ui-core-components (Typography sweep 1/13) - #30773
refactor(ui): migrate utils/* Typography to ui-core-components (Typography sweep 1/13)#30773chirag-madlani wants to merge 5 commits into
Conversation
…sions The antd-typography-to-core codemod only checked for a plain, unaliased `Typography` specifier when deciding whether core already imports Typography. It missed an existing `Typography as CoreTypography` alias, so on a fully-converted file it pushed a second `Typography` specifier onto the same import (utils/IngestionUtils.tsx repro), instead of reusing the alias already in scope. Fixed by resolving the existing core-import local name (plain or aliased) once, up front, and reusing it for both the fully-converted and partial/alias code paths; a new specifier is only introduced when no core import exists at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… 1/13) Chunk 1 of the antd Typography -> ui-core-components migration (docs/antd-migration/typography.md): 32 leaf files under openmetadata-ui/src/main/resources/ui/src/utils regenerated with the fixed antd-typography-to-core codemod. All 32 convert cleanly with no hand-finish skips. Also includes two fixes surfaced by the verification gates rather than the codemod itself: - utils/NavbarUtils.test.tsx and utils/CSV/CSV.utils.test.tsx asserted against / mocked the old antd Typography shape; updated to the core component so their tests keep passing. - Widened ui-core-components' TypographyProps (href/target/rel) so the as="a" shape produced by Typography.Link conversions type-checks; these props already reached the DOM at runtime via prop spreading, this only corrects the type surface to match, additive-only. Co-Authored-By: Claude Fable 5 <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 |
github-actions
Bot
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <Divider className="self-center m-x-sm" type="vertical" /> | ||
| <div className="d-flex items-center text-xs"> | ||
| <Typography.Link href={href} style={{ fontSize: '12px' }}> | ||
| <Typography as="a" href={href} style={{ fontSize: '12px' }}> |
There was a problem hiding this comment.
The migrated as="a" elements here and for pipelineDetails.sourceUrl inherit the core prose link color, which is gray rather than Ant Design's distinct link color, making these source links harder to distinguish from surrounding text. Add an explicit link-color style consistent with the rest of the application.
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!
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| <Typography | ||
| className="entity-title text-link-color font-medium m-r-xss" | ||
| data-testid="entity-title" | ||
| ellipsis={{ tooltip: true }}> | ||
| {title} | ||
| </Text> | ||
| </Typography> |
There was a problem hiding this comment.
⚠️ Bug: ellipsis tooltip nests a inside anchors
The core Typography renders ellipsis={{ tooltip: true }} through Tooltip/TooltipTrigger, and TooltipTrigger is a react-aria AriaButton (<button>). In EntitySummaryPanelUtils.getTitle these converted <Typography ellipsis={{ tooltip: true }}> titles sit directly inside a react-router <Link> (<a>), producing invalid <a><button>...</button></a> markup and an interactive button that can intercept the press so the title no longer navigates. antd's Typography.Text previously rendered a plain <span> with a hover tooltip and left navigation intact. Verify click-through on dashboard/entity summary titles; if broken, drop the ellipsis-tooltip on the linked title (use CSS truncation) or keep these specific sites on antd.
Was this helpful? React with 👍 / 👎
| {displayText} | ||
| <Typography.Text className="m-l-xs text-xs" type="secondary"> | ||
| <Typography className="m-l-xs text-xs" color="secondary"> | ||
| {fqn} | ||
| </Typography.Text> | ||
| </Typography> |
There was a problem hiding this comment.
💡 Edge Case: Text→core Typography changes inline spans to block
The core Typography always wraps its element in a block-level <div className="prose ...">, whereas antd Typography.Text renders a bare inline <span>. Conversions used in inline contexts therefore change from inline to block flow — e.g. SearchUtils.getSuggestionElement where the converted <Typography> follows {displayText} inline, and the many Transi18next renderElement={<Typography .../>} fragments in FeedUtils that are meant to wrap a portion of a sentence. This can push text onto its own line. Verify these render on one line visually; if not, these sites need an inline element or should stay on antd.
Was this helpful? React with 👍 / 👎
| <Typography as="h5" size="text-md" style={titleStyles}> | ||
| {timestamp} | ||
| </Typography.Title> | ||
| </Typography> |
There was a problem hiding this comment.
💡 Quality: Title→Typography conversion drops default bold weight
antd Typography.Title renders bold heading text by default. The codemod maps Typography.Title level={5} to <Typography as="h5" size="text-md"> (size only, no weight), so in DataInsightChartUtils.CustomTooltip the tooltip title loses its bold styling. If bold is intended, add weight="semibold" (or the appropriate weight) either at this call site or in the codemod's Title branch so heading emphasis is preserved across the sweep.
Preserve the bold heading weight lost from Typography.Title.:
<Typography as="h5" size="text-md" weight="semibold" style={titleStyles}>
{timestamp}
</Typography>
Was this helpful? React with 👍 / 👎
❌ UI Checkstyle Failed❌ Antd + Less Deprecation GuardA new Affected filesopenmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.tsx: import { Button, Checkbox, MenuProps, Radio, Space } from 'antd'; 🔍 ESLint findings in this PR's files — ESLint report could not be read — see the job log.Errors block the build. Warnings do not yet — they are rules whose backlog is still No machine-readable report was produced. Fix locally (fast - only checks files changed in this branch): make ui-checkstyle-changed |
🔴 Playwright Results — workflow failedValidated commit ✅ 779 passed · ❌ 1 failed · 🟡 1 flaky · ⏭️ 9 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ❌ unmet · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 1h 5m 27s ⏱️ Max setup 3m 2s · max shard execution 22m 1s · max shard-job elapsed before upload 26m 4s · reporting 5s 🌐 199.22 requests/attempt · 2.65 app boots/UI scenario · 30.96% common-shard skew Optimization targets still in progress:
Genuine Failures (failed on all attempts)❌
|
…-migration/typography-sweep-1
ClassificationUtils.tsx and IngestionListTableUtils.tsx now render the core Typography component instead of antd's Typography.Text. Two test suites mock '@openmetadata/ui-core-components' wholesale but didn't stub Typography, so it resolved to undefined at render time and broke ClassificationDetails.test.tsx and TestSuitePipelineTab.test.tsx in CI's full-suite jest run (our local `jest src/utils` run never touched these files). Add the same Typography stub already used in CSV.utils.test.tsx. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The incident-manager page renders seeded test-case rows whose names, table names, and "Last Updated" timestamps differ per CI run; the table's auto-width columns then shift the whole layout to fit that per-run content, so no committed baseline can be stable. Same class of non-determinism as 'roles', already dropped. Verified this is pre-existing and unrelated to this sweep: none of the 32 utils files touched by the sweep render anywhere in the IncidentManager component tree (checked the full import graph from IncidentManagerPage down), and the incident-manager baseline also fails intermittently on unrelated, concurrent PRs that never touch those files (e.g. runs 91219477124 "fix async-polling bugs in EntityExportModalProvider" and 91193942898 "Fix the flakiness in SearchRBAC test"). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Jest test CoverageUI tests summary
|
|
|
Superseded by #30780. Verified that this branch's head is fully contained in |
Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source
Summary
First real migration sweep of Wave 1 (#30565, epic #30570). Converts 32 files under
src/utilsfrom antdTypographyto@openmetadata/ui-core-components, driven by the codemod and mapping guide landed in #30721.utils/*was chosen as chunk 1 deliberately: pure render-helper functions rather than page shells, so the mechanical mapping gets validated end-to-end with a contained blast radius before the sweep moves into shared components and pages.Three commits:
fix(tooling)— codemod bug found by running it for real: the transform only detected a plain pre-existing coreTypographyimport and missed an existing alias, emitting a duplicate specifier (Typography as CoreTypography, Typography) inIngestionUtils.tsx. Now reuses whichever local name is already in scope. Regression tests added (TDD red→green; suite 37→39).refactor(ui)— the 32 regenerated files, plus two test files that encoded the old antd shape (NavbarUtils.test.tsxidentity assertion;CSV.utils.test.tsxwhose manual core-components jest mock lacked aTypographystub).TypographyPropswidening in core (href/target/rel) —Typography.Link → as="a"conversions are valid at runtime (props spread onto theaselement) but didn't typecheck, sinceHTMLAttributesexcludes anchor attrs. Fixed once at the type level because every later chunk with aTypography.Linkhits it.Verification
tsc --noEmit(34 changed files)jest src/utilsui-checkstyle:changedtsc0 errors,typography.test.tsx7/7Ledger: antd
Typographyin openmetadata-ui 421 → 388 (−33). 12 chunks remain (map in the sweep planning doc); zero hand-finish warnings fired in this chunk.Fixes #30772
🤖 Generated with Claude Code
Greptile Summary
This PR performs the first utility-layer Typography migration sweep.
src/utilswith the shared core Typography component.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (2): Last reviewed commit: "test(ui): drop non-deterministic inciden..." | Re-trigger Greptile