Skip to content

feat(ui-core): Added size and weight props to Typography component#26601

Merged
karanh37 merged 5 commits intomainfrom
core-ui-typography
Mar 20, 2026
Merged

feat(ui-core): Added size and weight props to Typography component#26601
karanh37 merged 5 commits intomainfrom
core-ui-typography

Conversation

@Rohit0301
Copy link
Contributor

@Rohit0301 Rohit0301 commented Mar 19, 2026

Describe your changes:

Fixes

I worked on ... because ...

Screen.Recording.2026-03-19.at.4.55.27.PM.mov

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • New props:
    • Added size prop with 12 typography scale options (text-xs to display-2xl)
    • Added weight prop with 4 font weight variants (regular, medium, semibold, bold)
  • Component improvements:
    • Refactored conditional rendering logic; as prop now defaults to "p"
    • Exported TypographySize and TypographyWeight types for external use
  • Stories:
    • Added comprehensive "All Variants" story showcasing all size and weight combinations in table format

This will update automatically on new commits.

@Rohit0301 Rohit0301 self-assigned this Mar 19, 2026
@Rohit0301 Rohit0301 added the safe to test Add this label to run secure Github workflows on PRs label Mar 19, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026

🟡 Playwright Results — all passed (14 flaky)

✅ 3395 passed · ❌ 0 failed · 🟡 14 flaky · ⏭️ 183 skipped

Shard Passed Failed Flaky Skipped
✅ Shard 1 455 0 0 2
✅ Shard 2 305 0 0 1
🟡 Shard 3 669 0 6 33
🟡 Shard 4 684 0 4 41
✅ Shard 5 672 0 0 73
🟡 Shard 6 610 0 4 33
🟡 14 flaky test(s) (passed on retry)
  • Features/BulkImport.spec.ts › Database (shard 3, 1 retry)
  • Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › EditAll User: Complete export-import-validate flow (shard 3, 1 retry)
  • Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit icon on incidents (shard 3, 1 retry)
  • Features/DataQuality/TestCaseResultPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit action on test case (shard 3, 1 retry)
  • Features/Glossary/GlossaryP3Tests.spec.ts › should handle multiple rapid API calls (shard 3, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Flow/ObservabilityAlerts.spec.ts › Alert operations for a user with and without permissions (shard 4, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 4, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContractsSemanticRules.spec.ts › Validate Description Rule Is_Set (shard 4, 1 retry)
  • Pages/Glossary.spec.ts › Column dropdown drag-and-drop functionality for Glossary Terms table (shard 6, 1 retry)
  • Pages/HyperlinkCustomProperty.spec.ts › should display URL when no display text is provided (shard 6, 1 retry)
  • Pages/ODCSImportExport.spec.ts › Multi-object ODCS contract - object selector shows all schema objects (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@karanh37 karanh37 merged commit 45b2465 into main Mar 20, 2026
21 checks passed
@karanh37 karanh37 deleted the core-ui-typography branch March 20, 2026 05:53
@gitar-bot
Copy link

gitar-bot bot commented Mar 20, 2026

Running post-merge workflows

Code Review ⚠️ Changes requested 1 resolved / 2 findings

Adds size and weight props to the Typography component while fixing invalid HTML from wrapping block elements with default paragraph tags. However, otherProps no longer spread on the outer div when no as prop is provided, which is a breaking change that needs addressing.

⚠️ Bug: otherProps no longer spread on outer div, breaking change

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:88-91

In the old implementation, when no as prop was provided, otherProps (e.g. id, data-testid, onClick, style, etc.) were spread on the outer <div>. The new code always spreads otherProps on the inner <Component> element, and the outer <div> receives no extra props at all.

This is a subtle breaking change: any caller that relied on HTML attributes (like data-testid, role, aria-*, style, event handlers) being applied to the outermost element will find those attributes have moved to a nested element. This can break test selectors, accessibility trees, and event handling.

✅ 1 resolved
Bug: Default as="p" wraps block elements, producing invalid HTML

📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:73 📄 openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:88-91 📄 openmetadata-ui-core-components/src/main/resources/ui/src/stories/Typography.stories.tsx:30-41
The previous implementation rendered children directly inside the <div class="prose"> wrapper when no as prop was given. The new code defaults as to "p", so every call site without an explicit as prop now gets an implicit <p> wrapper.

This causes two problems:

  1. Invalid HTML in the Default story: The Default story passes <h1> and <p> as children without an as prop. With the new default, this renders <p><h1>...</h1><p>...</p></p>, which is invalid HTML — <p> cannot contain block-level elements. Browsers will auto-close the outer <p>, producing broken/unexpected DOM.
  2. Subtle layout regression for existing callers: At least 4 call sites in the main app omit the as prop and previously had their children rendered directly inside the <div>. They now silently gain a <p> wrapper, which introduces block-level margin/padding from the prose styles and changes the DOM structure.

Consider defaulting to as="div" or as="span" (matching the previous behavior of no inner wrapper more closely), or removing the default entirely and only rendering a wrapper element when as is explicitly provided.

🤖 Prompt for agents
Code Review: Adds size and weight props to the Typography component while fixing invalid HTML from wrapping block elements with default paragraph tags. However, `otherProps` no longer spread on the outer div when no `as` prop is provided, which is a breaking change that needs addressing.

1. ⚠️ Bug: `otherProps` no longer spread on outer div, breaking change
   Files: openmetadata-ui-core-components/src/main/resources/ui/src/components/foundations/typography.tsx:88-91

   In the old implementation, when no `as` prop was provided, `otherProps` (e.g. `id`, `data-testid`, `onClick`, `style`, etc.) were spread on the outer `<div>`. The new code always spreads `otherProps` on the inner `<Component>` element, and the outer `<div>` receives no extra props at all.
   
   This is a subtle breaking change: any caller that relied on HTML attributes (like `data-testid`, `role`, `aria-*`, `style`, event handlers) being applied to the outermost element will find those attributes have moved to a nested element. This can break test selectors, accessibility trees, and event handling.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants