Skip to content

refactor(ui): complete antd Typography migration to ui-core-components - #30780

Merged
chirag-madlani merged 35 commits into
antd-migration/wave-1from
antd-migration/typography-sweep-rest
Aug 2, 2026
Merged

refactor(ui): complete antd Typography migration to ui-core-components#30780
chirag-madlani merged 35 commits into
antd-migration/wave-1from
antd-migration/typography-sweep-rest

Conversation

@chirag-madlani

@chirag-madlani chirag-madlani commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Completes the Typography sweep for openmetadata-ui (part of #30565, AntD → @openmetadata/ui-core-components migration program).

  • Scope: ~442 files changed across the sweep, split into chunks 2–13 as separate commits (plus this final hand-finish commit) so the change is bisectable if a visual regression surfaces later. Chunk 1 (utils/*) landed in an earlier PR.
  • Ledger: openmetadata-ui's Typography row in docs/antd-migration/LEDGER.md goes 422 → 0 (baseline count per docs/antd-migration/typography.md). Verified by re-running tooling/antd-migration/ledger.mjs against this branch — the Typography row is now absent entirely.
  • This PR's own contribution: hand-finishes the four files the codemod could only partially convert (AlertDetails.component.tsx, ReindexFailures.component.tsx, AdminPermissionDebugger.component.tsx, UserPermissions.component.tsx), which had no core equivalent for antd's code, copyable, and ellipsis.expandable. Resolved per the mapping guide's "no direct equivalent" section:
    • Typography.Text codeTypography as="code" with the rounded/mono/bg-secondary className idiom already established in ContextRuleCard.component.tsx.
    • Typography.Text/Paragraph copyableTypography composed with the existing CopyToClipboardButton in a flex row; ellipsis.expandable dropped in favor of rows: 2 (the cell already sits inside a Tooltip showing the full text, so the expand affordance was redundant).
    • Also fixed a regression this hand-edit introduced: core Typography always wraps content in an outer div.prose regardless of as, so nesting the new as="code" Typography inside the file's pre-existing as='p' Typography produced an invalid <div>-in-<p> (a validateDOMNesting warning not caught by any existing test assertion). Fixed by switching the outer wrapper to as="div".

Approved mapping decisions (docs/antd-migration/typography.md)

  • Typography.Title level={N}as="hN" + size via the approved LEVEL_SIZE_MAP (1→display-sm, 2→display-xs, 3→text-xl, 4→text-lg, 5→text-md). Level 5 dominates real usage (>70%).
  • type="secondary"/"success"/"warning"/"danger"color="secondary"/"success"/"warning"/"danger" (core color prop landed in main ahead of the bulk sweep).
  • strongweight="bold" (literal-only; dynamic strong={expr} was a codemod skip, hand-resolved where it occurred).
  • underlineclassName gets tw:underline appended.
  • Typography.TextTypography (as="span" default); Typography.ParagraphTypography as="p"; Typography.LinkTypography as="a".

Codemod vs. hand-finish

The codemod (tooling/antd-codemods/transforms/antd-typography-to-core.js) handled the mechanical prop/import rewrites above across chunks 2–13. Hand-finished on top of the codemod:

  • Bare <Typography> wrappers (codemod deliberately skips these — antd's bare form renders an <article>, core's default is span; each needed the intended element verified by hand): BlockEditor/*, CustomStatistic.tsx, TestCaseIncidentManagerStatus.component.tsx, SuccessScreen.tsx, SummaryTagsDescription.component.tsx, DataInsightHeader.component.tsx, ProfilerObjectFieldTemplate.tsx, MetricExpression.tsx, TourEndModal.tsx, Sso*FieldTemplate.tsx, ErrorPlaceHolderIngestion.tsx, WorkflowArrayFieldTemplate.tsx.
  • Dynamic Title level={expr} and type={expr}/unsupported type values (codemod skips, no fixed value to look up at codemod time).
  • The code/copyable/ellipsis.expandable hard gaps in this PR's four files (above).
  • Two real bugs found and fixed in UserProfileIcon.component.tsx and PersonaDetailsCard.tsx: core Typography's ellipsis={{ tooltip: true }} renders a real <button> trigger (via TooltipTrigger), which was nested inside another interactive element — a dropdown-trigger Button in one case, a whole-card onClick in the other. This is invalid HTML (button-in-button) and silently swallowed the parent's click. Fixed by switching to a plain ellipsis boolean + native title attribute, which preserves the hover-tooltip truncation without the interactive nesting.
  • PageHeader.interface.ts fix: titleProps/subHeaderProps were still typed against antd's Typography/Paragraph props after PageHeader.component.tsx itself was converted to core Typography (sweep chunk 4/13). antd's ParagraphProps inherits a generic HTML color?: string from HTMLAttributes, which isn't assignable to core Typography's narrower color?: TypographyColor union, so {...subHeaderProps} stopped type-checking once the component switched. Retyped both props against core's TypographyProps — the one genuine new tsc error the sweep introduced, isolated by diffing against the pre-sweep baseline.

Gate results

  • tsc --noEmit: 0 new errors (523 pre-existing baseline errors, unrelated to this sweep, unchanged before/after — isolated by diffing against origin/antd-migration/wave-1).
  • jest, full suite: Test Suites: 3 skipped, 1155 passed, 1155 of 1158 total / Tests: 52 skipped, 13637 passed, 13689 total. 0 failures.
  • eslint / prettier / organize-imports: clean on this PR's own hand-finish commit (one import-order violation and one formatting nit introduced by the hand-edit were caught and fixed before commit; all remaining eslint findings across the four touched files are pre-existing and unrelated, confirmed by diffing lint output against the pre-edit tree).

Notes

  • The Collate-side sweep (collate-ui, collate-local-webserver) is tracked as a separate PR in the openmetadata-collate repo.
  • Every converted call site carries DOM-wrapper layout-regression risk per the mapping guide (core Typography always renders inside div.prose) — the visual-regression baselines under the collate visual project should be checked against the affected pages before merging past this PR into main.

Test plan

  • tsc --noEmit — 0 new errors vs. pre-sweep baseline
  • jest full suite — 1155/1158 suites, 13637/13689 tests, 0 failures
  • eslint / prettier / organize-imports clean on the hand-finish commit
  • Ledger regenerated — Typography row absent for openmetadata-ui
  • Visual-regression baselines reviewed for pages touched by this sweep (tracked separately, see Notes)

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Fixes #30796

chirag-madlani and others added 22 commits July 31, 2026 21:56
…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>
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>
…e-components (sweep 2/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/common (AsyncSelectList
through ManageButtonContentItem, 26 leaf dirs / 40 files).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-components (sweep 3/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/common (NoOwner through
UserTeamSelectableList, 21 leaf dirs / 22 files).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-components (sweep 4/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components across a grab-bag of small, isolated
feature components: APIEndpoint, AuditLog, Certification,
Classifications, DataAssetRules, Domain, Learning, NavBar, PageHeader,
ProfileCard, SearchDropdown, Tag, Topic, UploadFile,
WorkflowDefinitions, AppBar, Container, DataAssets, DataProducts,
ExploreV1, NotificationBox, Pipeline, Suggestions, TestLibrary,
Visualisations, Announcement (26 dirs / 38 files).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dgeCenter/SettingsSso/SearchSettings/DataInsight Typography to ui-core-components (sweep 5/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/{Dashboard,Metric,
ServiceInsights,MlModel,KnowledgeCenter,SettingsSso,SearchSettings,
DataInsight} (36 files).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aphy to ui-core-components (sweep 6/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/{DriveService,Glossary,
Alerts,DataQuality} (31 files). DataQuality and Glossary are
entity-detail-page building blocks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ponents (sweep 7/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/{Entity,Modals,Explore}
(37 files). Entity is shared across many entity-detail pages.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ponents (sweep 8/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/{DataContract,Database}
(28 files).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…omponents (sweep 9/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/ActivityFeed (18 files).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nts (sweep 10/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/MyData (30 files),
including Widgets/ and CustomizableComponents/ (landing-page surface).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nents (sweep 11/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in components/Settings (36 files).
Concentrates 4 of the hardest hand-finish sites (copyable/code props),
handled separately per the migration guide.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (sweep 12/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components across 33 single-file page dirs plus
Configuration, DataInsightPage, KPIPage, LoginPage, RolesPage (39
files). Page shells carry the highest visual-regression exposure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…PoliciesPage/TasksPage Typography to ui-core-components (sweep 13/13)

Mechanical codemod conversion of antd Typography usages to
@openmetadata/ui-core-components in pages/{TaskFormSettingsPage,
SignUp,TableDetailsPageV1,PoliciesPage,TasksPage} (23 files). TasksPage
is the single largest page directory, kept whole.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolve the sweep's remaining hand-finish worklist per
docs/antd-migration/typography.md:

- BlockEditor/* (BubbleMenu, AttachmentPlaceholder, HashList, MentionList,
  SlashCommandList): bare antd `<Typography>` wrappers with no automated
  coverage, converted directly to core Typography.
- CustomStatistic.tsx: `Text type={expr}` (unsupported-type skip) resolved
  to `color={expr}` since both ternary branches are allowed values.
- TestCaseIncidentManagerStatus.component.tsx, SuccessScreen.tsx,
  SummaryTagsDescription.component.tsx, DataInsightHeader.component.tsx:
  bare `<Typography>` usage mixed with converted sub-components; dropped
  the antd import and renamed the `CoreTypography` alias back to
  `Typography`.
- ProfilerObjectFieldTemplate.tsx, MetricExpression.tsx, TourEndModal.tsx,
  Sso{RolesSelectField,ConfigurationFormArrayFieldTemplate}.tsx,
  ErrorPlaceHolderIngestion.tsx, WorkflowArrayFieldTemplate.tsx: bare
  `<Typography>` (zero-conversion, not counted in the codemod's 410)
  converted by hand.
- UserProfileIcon.component.tsx, PersonaDetailsCard.tsx: core
  `ellipsis={{ tooltip: true }}` renders a real `<button>` trigger
  (TooltipTrigger), which is invalid nested inside another interactive
  element (a dropdown-trigger Button / whole-card onClick). Switched to
  plain `ellipsis` + a native `title` attribute at the two sites where
  this nesting broke click handling.
- TierCard.tsx: fixed a pre-existing `react/jsx-sort-props` prop-order
  violation surfaced by running eslint over the full changed-file set.

AlertDetails.component.tsx, ReindexFailures.component.tsx,
AdminPermissionDebugger.component.tsx, and UserPermissions.component.tsx
keep their partial `CoreTypography` alias — `code`/`copyable`/
`ellipsis.expandable` have no core equivalent yet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix the test-mock breakage class the Typography sweep repeatedly
triggers: a jest.mock('@openmetadata/ui-core-components', ...) factory
that omits Typography now returns undefined for a component that starts
importing it, crashing with "Element type is invalid" ("Check the
render method of ..."). Fixed in TableQueries.test.tsx,
ColumnDetailPanel.test.tsx, LineageTabContent.test.tsx,
ExploreSearchCard.test.tsx, TestSuitesTable.test.tsx, TestSuites.test.tsx
by adding the established Typography stub:
({ as: Component = 'span', children, ...props }) => <Component
{...props}>{children}</Component>.

Also fixes a second, narrower breakage class: stale antd
`jest.mock('antd', ...)` stubs for `Typography.Text`/`.Paragraph` that
inject a `data-testid` no longer used once the underlying component
switched to core Typography (DomainsSection, DataQualitySection,
DataProductsSection, OwnersSection, TagsSection, LineageTabContent).
Removed the dead mocks and updated assertions to query by the real
rendered class/text instead of the removed synthetic testid.

IncidentManagerPage.test.tsx and WidgetHeader.test.tsx had an existing
Typography stub that dropped all props except `children` (or dropped
`as`/`data-testid`); widened both to the same forwarding pattern so
`data-testid="heading"`/`"sub-heading"` and the `tw:truncate` ellipsis
class survive the mock.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PageHeader.interface.ts still typed titleProps/subHeaderProps as antd's
Typography/Paragraph props after PageHeader.component.tsx was converted
to core Typography (sweep 4/13). antd's ParagraphProps inherits a
generic HTML `color?: string` from HTMLAttributes, which isn't
assignable to core Typography's narrower `color?: TypographyColor`
union, so spreading `{...subHeaderProps}` onto the core component no
longer type-checked. Point both props at core's TypographyProps
instead — this was the one genuine new tsc error introduced by the
sweep (isolated by diffing against the pre-sweep baseline).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finishes the sweep's hand-finish worklist: AlertDetails, ReindexFailures,
AdminPermissionDebugger, and UserPermissions kept a partial CoreTypography
alias because their `code`/`copyable`/`ellipsis.expandable` usages had no
core equivalent. Resolves the gaps directly:

- `Typography.Text code` -> `CoreTypography as="code"` with the
  rounded/mono/bg-secondary className idiom already used by
  ContextRuleCard.component.tsx.
- `Typography.Text/Paragraph copyable` -> `CoreTypography` composed with
  the existing `CopyToClipboardButton` in a flex row; the `expandable`
  ellipsis option is dropped in favor of `rows: 2` since the cell already
  sits inside a Tooltip showing the full text.
- Drop the now-unused `Typography` antd import/destructure from all four
  files.

Also fixes a regression the hand-edit introduced in AlertDetails: core's
Typography always wraps its content in an outer `<div className="prose">`
regardless of `as`, so nesting the new `as="code"` Typography inside the
pre-existing `as='p'` Typography produced an invalid `<div>`-in-`<p>`
(validateDOMNesting warning, not caught by any existing assertion). Switched
the outer wrapper to `as="div"`.

openmetadata-ui's Typography ledger row now reads 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@chirag-madlani
chirag-madlani requested review from a team and karanh37 as code owners July 31, 2026 21:35
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review. (449 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 skip-pr-checks label.

@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Jul 31, 2026
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/FeedUtils.tsx
chirag-madlani and others added 3 commits August 1, 2026 20:02
Typography always wraps its children in a block-level div.prose,
regardless of the `as` prop. Rendering the `(fqn)` hint as Typography
inside the suggestion Link/Button broke the single-line layout,
pushing the fqn onto a second line and shifting the Button's click
point off the anchor's actual line boxes. GlobalSearchSuggestions.spec.ts
"Navigate to column from column suggestion" timed out waiting for the
resulting navigation because the click never reached the Link.

Use a plain span (this Typography added nothing but a className) to
keep the text inline and preserve the original click target geometry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…owed

core Typography's `ellipsis={{ tooltip: true }}` renders its content
inside a react-aria TooltipTrigger `<button>`. react-aria's usePress
stops native click propagation by default (see
@react-aria/interactions usePress.js), so a click landing on that
button never bubbles to an ancestor's onClick.

Two call sites in this file relied on exactly that bubbling:
- the persona-name Typography inside `data-testid="persona-label"`,
  whose *ancestor* div.onClick performs the actual persona switch;
- the "View Profile" Typography inside a react-router `<Link>`.

Clicking either silently no-op'd instead of switching persona or
navigating. This is why SettingsNavigationPage.spec.ts "should handle
multiple items being hidden at once" saw app-bar-item-explore still
visible: `getByRole('menuitem', { name: persona.displayName }).click()`
never actually invoked `handleSelectedPersonaChange`, so the persona
whose nav config was edited was never selected.

Apply the same fix already used for `default-persona` in this file:
plain `ellipsis` truncation plus a native `title` attribute, which
preserves the hover-tooltip text without adding a click-swallowing
interactive element.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hange

Same react-aria click-swallowing hazard as UserProfileIcon: the test
name and test-definition name in each test-case card use
`ellipsis={{ tooltip: true }}`, which wraps them in a react-aria
TooltipTrigger `<button>` that stops click propagation by default.
The card's selection handler lives on the outer `<Space
onClick={() => handleCardClick(test)}>`, so clicking the (often
truncated) test name inside the button no longer selected the card.

This broke test-case selection in the "Add Test Case" list shared by
TestSuite.spec.ts "Logical TestSuite", TestSuiteMultiPipeline.spec.ts
"TestSuite multi pipeline support", and TestSuiteDetailsPage.spec.ts
"Add test case modal on Test Suite details page - filters and select"
- all three clicked `getByTestId(testCaseName)` inside
`test-case-selection-card` and then hung waiting for the resulting
test-suite create/deploy API calls that never fired because no test
case was ever actually selected.

Attach the same onClick directly to both Typography elements so
selection no longer depends on the click bubbling past the
click-swallowing wrapper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
chirag-madlani and others added 3 commits August 2, 2026 02:10
Resolves visual-harness conflicts in favour of wave-1: the harness repair
(#30798) restored the pages this branch had dropped as a stopgap (roles,
teams, data-quality, incident-manager, landing-page x2) by masking their
volatile regions instead, and clipped incident-manager to page chrome. That
supersedes the drops here, so wave-1's staticPages.spec.ts and all its
CI-rendered baselines win.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Typography always rendered its content inside a `<div className="prose">`.
antd's `Typography.Text` rendered an inline `<span>`, so every mechanically
converted call site became block-level: text embedded mid-sentence wrapped onto
its own line, and a nested Typography produced a `<div>` inside a `<span>` —
invalid DOM that trips React's validateDOMNesting.

`styles/typography.css` applies its real rules through a *descendant* selector
(`.prose :not(...)`), and every rule in it is gated on an element type — p,
h1-h6, ol, ul, li, blockquote, a, code, pre, img, figure, table. For those the
wrapper is load-bearing; moving `prose` onto the element would stop the rule
matching (a `p` would silently lose its margins).

`span` and `div` are targeted by no such rule. There the wrapper contributes
only the element-level `.prose` layer — the `--tw-prose-*` vars plus `color`,
`font-size` and `line-height`, all inherited properties — so setting `prose`
directly on the element yields an identical computed style on the text while
dropping the spurious block box.

Render the element directly in that case. Ellipsis keeps the wrapper (it carries
the truncation classes) and so does a non-default quote variant (also styled via
a descendant selector). Deliberately a small allowlist: anything unlisted is
unchanged.

Verified with the full consumer suite: 1155 jest suites / 13637 tests, no
failures.
Follow-ups from the code review on the antd Typography sweep:

- SignInPage: `md:tw:gap-10` is not a valid Tailwind v4 prefix-mode class and
  generated no CSS, so the breakpoint spacing the deleted `.less` provided was
  silently lost. The prefix must lead the variant chain (`tw:md:gap-10`).

- SignInPage: `.forgot-password-link`'s `flex: 1` only applies when the anchor is
  a direct flex child of antd's `inline-flex` label. Typography wraps `as="a"` in
  a block `div.prose`, breaking that. Own the row layout locally instead.

- PageHeader: `{...titleProps}` / `{...subHeaderProps}` were spread *before* a
  hardcoded `className`, so a caller-supplied className was silently dropped.
  Merge them instead.

- AddTestCaseList: drop the `onClick` handlers that were re-wired onto the
  Typography elements. They sit inside the react-aria TooltipTrigger button, so
  today they never fire; and once that button stops swallowing propagation
  (#30804) they would fire *in addition to* the row-level handler on the
  <Space>, and since `handleCardClick` toggles, the two would cancel out.
  Selection is left to the single row-level handler.

- DataAssetsHeaderExtraInfo / MarketPlaceAppDetails: both used a default
  (inline) Typography purely as a layout container around block content. Mark
  them `as="div"`, which is what they were structurally, and which clears the
  last remaining invalid nesting — a scan of all 591 Typography files now
  reports none.
@chirag-madlani

Copy link
Copy Markdown
Collaborator Author

Addressed the review in two commits — eec96c9 (core) and d024312 (call sites).

Findings 1 + 2 share one root cause, so they're fixed once in core rather than at 442 call sites.

typography.css works in two layers: .prose:not(...) on the element itself sets only the --tw-prose-* vars plus color/font-size/line-height, while .prose :not(...) — a descendant selector — carries every real typographic rule, each gated on an element type (p, h1-h6, ol, ul, li, blockquote, a, code, pre, img, figure, table).

So the wrapper is genuinely load-bearing for those types — putting prose on a <p> directly would stop .prose p matching and silently drop its margins. But span and div are targeted by no rule in that block, so for them the wrapper contributes only inherited properties, and setting prose on the element gives an identical computed style. Those now render without a wrapper; ellipsis and non-default quote variants keep it. Deliberately a small allowlist.

A scan of all 591 files containing <Typography reports no remaining invalid nesting. The two that survived the core fix (DataAssetsHeaderExtraInfo, MarketPlaceAppDetails) were inline Typography used purely as layout containers and are now as="div".

Finding 5 — I did not apply the suggested fix. The diagnosis is right that the handler never fires today, but the proposed wrapper <div onClick> would introduce a different bug. handleCardClick is a toggle, and the row-level onClick on the <Space> is still attached. Once #30804 stops the TooltipTrigger swallowing propagation, both handlers fire on one click and cancel each other out — selection would appear frozen. Removed the re-wires instead and left selection to the single row-level handler. This makes the PR depend on #30804, which was already in the merge chain.

Findings 3 and 4 applied as suggested.

Note for baselines: this restores inline flow, so it legitimately changes rendering. The current visual-regression baselines were captured with the block wrapper, i.e. they encode the regression as expected. They will need regenerating from CI after #30804 and this land — the diffs are the fix, not a new break.

Verified: full consumer suite green (1155 suites / 13,637 tests), core suite 21/21, eslint clean.

…weep-rest

# Conflicts:
#	openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx
Dropping Typography's block `div.prose` wrapper for span/div lets former
`Typography.Text` call sites participate in inline flow again, which legitimately
changes rendering on three of the twenty harness pages.

Each diff was compared against its previous baseline before adoption rather than
adopted wholesale:

- applications: "Cache Warmup" no longer force-wraps onto two lines, so the
  `Disabled` badge sits inline beside the title instead of being pushed down;
  cards tighten by ~12px. The bold "Data Insights" descriptions are present in
  the old baseline too - pre-existing markdown, not a change here.
- landing-page / landing-page-sidebar-collapsed: the hero tightens, shifting the
  widget grid up ~10px. The large diff regions are the harness's own mask fills
  moving with it, not content changes.

These baselines had encoded the block-wrapper layout as expected, which is how
the regression escaped review in the first place. Captured from CI artifacts per
docs/antd-migration/README.md - never from a local run.
@sonarqubecloud

sonarqubecloud Bot commented Aug 2, 2026

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Aug 2, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 5 resolved / 5 findings

Completes the openmetadata-ui migration from AntD Typography to ui-core-components, clearing the ledger entry and fully resolving the remaining hand-edit integration bugs. No issues found.

✅ 5 resolved
Bug: Nested Typography emits invalid
-in- and breaks layout

📄 openmetadata-ui/src/main/resources/ui/src/components/Modals/DeployIngestionLoaderModal/DeployIngestionLoaderModal.tsx:52-66 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:127-141
Core Typography always renders its content inside a block-level <div className="prose"> wrapper regardless of the as prop (typography.tsx:193-199), whereas antd Typography.Text rendered an inline <span>. Where a Typography.Text was nested inside another Typography.Text, the mechanical conversion now produces a <div> nested inside a <span> — invalid DOM nesting that triggers React validateDOMNesting warnings and collapses the intended inline layout. DeployIngestionLoaderModal.tsx is a concrete case: the outer <Typography className="ingestion-deploy-rounder"> now wraps an inner <Typography className="flex-center h-full">, so the circular loader markup becomes <div><span><div><span>…, and the style/rounder classes that positioned the circle are now buried under an extra block div. The PR only hand-fixed the div-in-p case in its four hand-finished files; codemod-converted nested cases like this were not audited. Verify nested Typography sites and switch inner/outer wrappers to as="div" or restructure so no block wrapper lands inside an inline element.

Edge Case: Inline Typography.Text→core Typography turns inline text into block divs

📄 openmetadata-ui/src/main/resources/ui/src/utils/FeedUtils.tsx:249-250 📄 openmetadata-ui/src/main/resources/ui/src/utils/FeedUtils.tsx:261-262 📄 openmetadata-ui/src/main/resources/ui/src/utils/FeedUtils.tsx:268-269 📄 openmetadata-ui/src/main/resources/ui/src/utils/FeedUtils.tsx:278-279 📄 openmetadata-ui/src/main/resources/ui/src/pages/LoginPage/SignInPage.tsx:234-244 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:193-199
Because core Typography unconditionally wraps output in <div className="prose"> (typography.tsx:193-199), every Typography.Text that was previously an inline <span> becomes a block-level element after conversion, even with the default as="span". This breaks call sites that relied on inline flow. For example FeedUtils.tsx passes renderElement={<Typography className="font-bold" />} into Transi18next, so the bold segment embedded mid-sentence in activity-feed headers now renders as a block div and wraps to its own line; SignInPage.tsx renders the "Password" label and "Forgot password" link (as='a') as two block divs that no longer sit inline. The PR notes acknowledge this DOM-wrapper regression risk, but the corresponding test-plan item (visual-regression baseline review) is still unchecked. Confirm these inline sites against visual baselines before merge and adjust where inline rendering is required.

Quality: PageHeader hardcoded className overrides caller titleProps/subHeaderProps

📄 openmetadata-ui/src/main/resources/ui/src/components/PageHeader/PageHeader.component.tsx:34-39 📄 openmetadata-ui/src/main/resources/ui/src/components/PageHeader/PageHeader.component.tsx:55-59
In PageHeader.component.tsx the spread {...titleProps} / {...subHeaderProps} is placed before the hardcoded className="heading m-b-0" / className="sub-heading" (lines 34-39, 55-59). Since a later className prop wins, any className a caller supplies via titleProps/subHeaderProps is silently dropped. With the props now typed as core TypographyProps, callers can still pass className, so this can quietly discard styling. If per-caller className overrides are intended, merge them (e.g. className={cx('heading m-b-0', titleProps?.className)}) instead of hardcoding after the spread.

Bug: Responsive gap uses wrong Tailwind v4 prefix order

📄 openmetadata-ui/src/main/resources/ui/src/pages/LoginPage/SignInPage.tsx:196
In Tailwind v4 prefix mode the tw: prefix must lead the variant chain (tw:md:gap-10), as used in 37 other call sites across the repo. The new md:tw:gap-10 is not a recognized class and generates no CSS, so the intended larger gap at the md breakpoint never applies — only tw:gap-6 (24px) takes effect at all widths, silently dropping the breakpoint spacing the deleted .less media queries provided. Change md:tw:gap-10 to tw:md:gap-10.

Bug: onClick re-wire is nested inside the propagation-stopping button

📄 openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx:650-655 📄 openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx:668-672 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:174-188
The comment states the react-aria TooltipTrigger <button> (rendered by core Typography when ellipsis={{ tooltip: true }} is set) stops click propagation, which is why the parent <Space onClick> no longer fires. But the newly added onClick is passed via otherProps to the inner <Component>, which core Typography renders inside that same button (<button><div.prose><Component/></button>, see typography.tsx:174-190). With React 18 root-level event delegation and react-aria's default shouldStopPropagation, the native click is halted at the button before it reaches the React root, so a React onClick on a descendant of the button may never fire — meaning clicking the test-case name still would not select the card. Verify this path in a real browser (Playwright), not just jsdom; if confirmed, attach the handler to an element rendered outside the Typography button (e.g. wrap the Typography in a <div onClick={() => handleCardClick(test)}>), or drop the ellipsis tooltip on these labels.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant