Skip to content

fix(ui): enhance FormItemLabel with customizable class names for label and beta badge#29733

Open
chirag-madlani wants to merge 3 commits into
mainfrom
chore/form-item-classname
Open

fix(ui): enhance FormItemLabel with customizable class names for label and beta badge#29733
chirag-madlani wants to merge 3 commits into
mainfrom
chore/form-item-classname

Conversation

@chirag-madlani

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

Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #

support classnames for better overrides

I worked on ... because ...

Type of change:

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

High-level design:

N/A — small change.

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Not applicable.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • 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.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Greptile Summary

This PR adds two optional className props — labelClassName and betaBadgeClassName — to FormItemLabel, enabling callers to override styles on the label span and beta badge independently. The component now wraps both elements with the classnames utility for conditional class merging, and also introduces a hardcoded base class form-item-label on the span.

  • FormItemLabelProps in Form.interface.ts gains labelClassName?: string and betaBadgeClassName?: string.
  • FormItemLabel.tsx imports classnames, destructures the two new props, and applies them via classNames(...) alongside the existing default classes.

Confidence Score: 5/5

Safe to merge — the change is purely additive, adding two optional className props with no effect on existing callers.

Both new props are optional with no defaults, so all existing usages are completely unaffected. The classnames merging logic is straightforward and the TypeScript interface change is backward-compatible. The only open question — already noted in a prior review comment — is whether organize-imports-cli reorders the new classnames import in CI; this is a cosmetic lint concern and not a logic risk.

No files require special attention.

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/components/common/Form/Form.interface.ts Adds two optional string props (labelClassName, betaBadgeClassName) to FormItemLabelProps; purely additive, backward-compatible change.
openmetadata-ui/src/main/resources/ui/src/components/common/Form/FormItemLabel.tsx Imports classnames, applies labelClassName and betaBadgeClassName props to the label span and Beta badge; import ordering of the new classnames entry may conflict with organize-imports-cli expectations.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[FormItemLabel] --> B{props}
    B --> C["label + labelClassName\n→ span.form-item-label"]
    B --> D{helperTextType === Tooltip\n&& helperText\n&& showHelperText}
    D -- Yes --> E[Tooltip + InfoCircleOutlined]
    D -- No --> F[nothing]
    B --> G{isBeta?}
    G -- Yes --> H["Badge\nm-l-xs + betaBadgeClassName"]
    G -- No --> I[nothing]
Loading
%%{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
    A[FormItemLabel] --> B{props}
    B --> C["label + labelClassName\n→ span.form-item-label"]
    B --> D{helperTextType === Tooltip\n&& helperText\n&& showHelperText}
    D -- Yes --> E[Tooltip + InfoCircleOutlined]
    D -- No --> F[nothing]
    B --> G{isBeta?}
    G -- Yes --> H["Badge\nm-l-xs + betaBadgeClassName"]
    G -- No --> I[nothing]
Loading

Comments Outside Diff (1)

  1. openmetadata-ui/src/main/resources/ui/src/components/common/Form/FormItemLabel.tsx, line 14-21 (link)

    P2 The classnames import is an external library but is placed after all local relative imports, which violates the project's import ordering convention (external → internal absolute → relative). The organize-imports-cli CI check will flag this and the PR will fail lint.

    Context Used: CLAUDE.md (source)

    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!

Reviews (2): Last reviewed commit: "Merge branch 'main' into chore/form-item..." | Re-trigger Greptile

Copilot AI review requested due to automatic review settings July 3, 2026 11:57
@chirag-madlani chirag-madlani requested a review from a team as a code owner July 3, 2026 11:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jul 3, 2026

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 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
63.26% (71236/112595) 46.33% (41422/89400) 47.56% (12710/26719)

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 1 failure(s), 21 flaky

✅ 4484 passed · ❌ 1 failed · 🟡 21 flaky · ⏭️ 37 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 441 0 2 16
🔴 Shard 2 804 1 5 8
🟡 Shard 3 805 0 4 7
🟡 Shard 4 810 0 3 5
🟡 Shard 5 855 0 2 0
🟡 Shard 6 769 0 5 1

Genuine Failures (failed on all attempts)

Features/BulkImport.spec.ts › Database (shard 2)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoContainText�[2m(�[22m�[32mexpected�[39m�[2m)�[22m failed

Locator: locator('.rdg-cell-details')
Timeout: 15000ms
�[32m- Expected  - 1�[39m
�[31m+ Received  + 1�[39m

�[2m  Array [�[22m
�[31m+   "#FIELD_REQUIRED: Field 1 is required;#FIELD_REQUIRED: Field 13 is required",�[39m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity updated",�[22m
�[32m-   "Entity created",�[39m
�[2m    "Entity created",�[22m
�[2m    "Entity updated",�[22m
�[2m    "Entity created",�[22m
�[2m  ]�[22m

Call log:
�[2m  - Expect "toContainText" with timeout 15000ms�[22m
�[2m  - waiting for locator('.rdg-cell-details')�[22m
�[2m    19 × locator resolved to 12 elements�[22m

🟡 21 flaky test(s) (passed on retry)
  • Flow/Tour.spec.ts › Tour should work from help section (shard 1, 2 retries)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: metric (shard 1, 1 retry)
  • Features/BulkImport.spec.ts › Database service (shard 2, 1 retry)
  • Features/BulkImport.spec.ts › Database Schema (shard 2, 1 retry)
  • Features/BulkImport.spec.ts › Table (shard 2, 1 retry)
  • Features/DataQuality/TableLevelTests.spec.ts › Table Column Name To Exist (shard 2, 1 retry)
  • Features/Glossary/GlossaryCRUDOperations.spec.ts › should create glossary with mutually exclusive enabled (shard 2, 1 retry)
  • Features/KnowledgeCenterList.spec.ts › Knowledge Center List - Verify Recently Viewed widget (shard 3, 1 retry)
  • Features/SearchExport.spec.ts › Export queues a background job and downloads from the jobs tray (shard 3, 1 retry)
  • Features/Table.spec.ts › should persist page size (shard 3, 1 retry)
  • Features/Tasks/TaskNavigation.spec.ts › navigating to /table/TASK-XXXXX should show 404 (invalid URL pattern) (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display domain and owner of deleted asset in suggestions when showDeleted is on (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Set & Update all CP types on chart (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for MlModel (shard 4, 1 retry)
  • Pages/Entity.spec.ts › User as Owner with unsorted list (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel.spec.ts › Should allow Data Steward to edit glossary terms for dashboardDataModel (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify Impact Analysis service filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab is NOT visible for pipelineService in platform lineage (shard 6, 1 retry)
  • Pages/ServiceEntity.spec.ts › Team as Owner Add, Update and Remove (shard 6, 1 retry)
  • Pages/TasksUIFlow.spec.ts › Create and reject tag task for Dashboard via UI (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

Copilot AI review requested due to automatic review settings July 3, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Jul 3, 2026

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

Enhances FormItemLabel with customizable class names for labels and beta badges, resolving the import ordering issue that would have failed lint checks.

✅ 1 resolved
Quality: Misplaced 'classnames' import breaks import ordering

📄 openmetadata-ui/src/main/resources/ui/src/components/common/Form/FormItemLabel.tsx:21
The new import classNames from 'classnames'; is added at line 21, after the local (relative) imports. The project's import-ordering convention (enforced by yarn organize-imports:cli / eslint) groups third-party package imports before relative imports and sorts them alphabetically. classnames is an external package and should appear in the third-party group (e.g., alphabetically between antd and react-i18next), not at the bottom after ./Form.interface. As-is this will fail the CI checkstyle/lint step. Move the import up:

import { InfoCircleOutlined } from '@ant-design/icons';
import { Badge, Tooltip } from 'antd';
import classNames from 'classnames';
import { useTranslation } from 'react-i18next';
import { GRAYED_OUT_COLOR } from '../../../constants/constants';
...
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

@sonarqubecloud

sonarqubecloud Bot commented Jul 3, 2026

Copy link
Copy Markdown

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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants