Skip to content

Mui switch, form label UI migration#26073

Open
Rohit0301 wants to merge 16 commits intomainfrom
mui-switch-autocomplete-ui-migration
Open

Mui switch, form label UI migration#26073
Rohit0301 wants to merge 16 commits intomainfrom
mui-switch-autocomplete-ui-migration

Conversation

@Rohit0301
Copy link
Contributor

@Rohit0301 Rohit0301 commented Feb 24, 2026

Describe your changes:

Fixes

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

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

  • UI component library migration: Unified FormItemLabel implementations and replaced MUISwitch with core components (Toggle, Badge, Tooltip) from @openmetadata/ui-core-components, eliminating dual Ant Design/Material-UI implementations
  • Switch component refactoring: Removed MUISwitch wrapper, changed FieldTypes.SWITCH_MUI to FieldTypes.UT_SWITCH, updated form rendering to use Toggle with valuePropName="isSelected"
  • Component consolidation: Moved FormItemLabel to dedicated directory using Tailwind CSS styling (tw: classes) and @untitledui/icons, standardized test IDs from mui-* to form-item-*
  • Import path updates: Updated 8 files importing from old Form/MUIFormItemLabel paths to new FormItemLabel location with consistent tooltip placement type (@react-types/overlays Placement)

This will update automatically on new commits.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 26, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.78% (56474/85850) 45.26% (29607/65413) 48.03% (8919/18568)

MOCK_TAGS_CATEGORY,
} from './TagsPage.mock';

jest.mock('@openmetadata/ui-core-components', () => {
Copy link

Choose a reason for hiding this comment

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

💡 Quality: Duplicated @openmetadata/ui-core-components mock across 7+ test files

The identical jest.mock('@openmetadata/ui-core-components', ...) block (with Toggle, Tooltip, TooltipTrigger, Badge, and createMuiTheme mocks) is copy-pasted across at least 7 test files:

  • TagsPage.test.tsx
  • TagsForm.test.tsx
  • FormItemLabel.test.tsx
  • TestSuitePipelineTab.test.tsx
  • EditTestCaseModal.test.tsx
  • EditTestCaseModalV1.test.tsx
  • TestCaseForm.test.tsx
  • StyleModal.test.tsx
  • TestDefinitionForm.test.tsx

This will become a maintenance burden — any API change to these components requires updating every copy. Consider extracting this into a shared mock file (e.g., __mocks__/@openmetadata/ui-core-components.tsx or a shared test utility) and importing it where needed.

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@gitar-bot
Copy link

gitar-bot bot commented Mar 5, 2026

🔍 CI failure analysis for 630e364: All Playwright CI failures (playwright-ci-postgresql runs 3/6 and 6/6) are pre-existing flaky tests unrelated to the PR's MUISwitch→Toggle and FormItemLabel component migrations.

Issue

Two Playwright CI jobs have failures, both with high pass rates and all failures unrelated to this PR.

playwright-ci-postgresql (6, 6): 1 failed, 5 flaky, 628 passed (97.4% pass rate)
playwright-ci-postgresql (3, 6): 1 failed, 10 flaky

Root Cause

All failures are pre-existing flaky E2E test issues caused by infrastructure and timing problems, not by this PR's component changes.

Common failure patterns across both jobs:

  1. Timeout errors — Tests hitting 60s–180s limits waiting for elements or API responses
  2. DOM selector instabilitygetByTestId('glossary-container') resolves to 8+ elements (strict mode violation)
  3. Undefined test data — Variables like displayName and fields[0] undefined during test setup
  4. API infrastructure issues — Socket hangs on DELETE /api/v1/glossaries and other endpoints
  5. Page/browser closed — Cascading failures after timeouts

Tests failing (job 6/6):

  • SearchIndexApplication.spec.ts — timeout after 180s
  • Glossary.spec.ts — drag/drop timing
  • ODCSImportExport.spec.ts — selector timing
  • Tag.spec.ts — tag operations timeout
  • Users.spec.ts — waiting for edit-displayName-button
  • VersionPages/EntityVersionPages.spec.tsCannot read properties of undefined (reading '0')

Tests failing (job 3/6):

  • BulkImport.spec.ts, GlossaryAssets.spec.ts, GlossaryHierarchy.spec.ts, GlossaryWorkflow.spec.ts, MultipleRename.spec.ts, OnlineUsers.spec.ts, GlossaryPermissions.spec.ts, ServiceEntityPermissions.spec.ts, AdvancedSearches.spec.ts

Why Unrelated to This PR

This PR modifies:

  • MUISwitchToggle component replacement
  • MUIFormItemLabelFormItemLabel component migration
  • Import path updates across 8 files

None of the failing tests involve Switch, Toggle, or FormItemLabel components. Zero references to these components appear in any error messages. The failures span Glossary, Permissions, BulkImport, SearchIndex, Users, Tags — all unrelated to form component styling.

Different tests fail in different job shards (random distribution), which is a hallmark of infrastructure flakiness rather than deterministic code issues. These same tests are known to fail intermittently across unrelated PRs.

Code Review 👍 Approved with suggestions 4 resolved / 6 findings

Mui switch and form label UI migration addresses placement type mismatches, copyright year updates, and valuePropName incompatibilities with Toggle components. Consider tightening prop spreading in the UT_SWITCH case to avoid unrecognized props, and consolidating the duplicated @openmetadata/ui-core-components mock across test files.

💡 Bug: Unrecognized props spread onto Toggle via switchRest

📄 openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:463 📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/tagFormFields.tsx:178

In the UT_SWITCH case, props is cast as SwitchProps and only disabled and onChange are destructured out. The remaining switchRest (which may include checked, label, size, and any extra keys like inputProps or initialValue from callers like getDisabledField) is spread onto the Toggle component.

Current callers pass props like inputProps, initialValue, data-testid, and className which are not valid Toggle props. While data-testid and className pass through harmlessly as HTML attributes, inputProps and initialValue are unrecognized and may trigger React warnings about unknown DOM attributes.

Additionally, {...switchRest} comes after the explicit label prop, so if any caller includes label in their props, it would silently override the muiLabel-derived label.

Consider destructuring all known props explicitly and only spreading validated rest props, similar to how other field types in this file handle their props.

Suggested fix
    case FieldTypes.UT_SWITCH: {
      const { disabled, onChange, size, ...rest } = props as SwitchProps;

      return (
        <Form.Item {...formProps} valuePropName="isSelected">
          <Toggle
            isDisabled={disabled}
            label={muiLabel as string}
            size={size}
            onChange={onChange}
          />
        </Form.Item>
      );
    }
💡 Quality: Duplicated @openmetadata/ui-core-components mock across 7+ test files

📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:44 📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx:21 📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormItemLabel/FormItemLabel.test.tsx:20 📄 openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.test.tsx:95

The identical jest.mock('@openmetadata/ui-core-components', ...) block (with Toggle, Tooltip, TooltipTrigger, Badge, and createMuiTheme mocks) is copy-pasted across at least 7 test files:

  • TagsPage.test.tsx
  • TagsForm.test.tsx
  • FormItemLabel.test.tsx
  • TestSuitePipelineTab.test.tsx
  • EditTestCaseModal.test.tsx
  • EditTestCaseModalV1.test.tsx
  • TestCaseForm.test.tsx
  • StyleModal.test.tsx
  • TestDefinitionForm.test.tsx

This will become a maintenance burden — any API change to these components requires updating every copy. Consider extracting this into a shared mock file (e.g., __mocks__/@openmetadata/ui-core-components.tsx or a shared test utility) and importing it where needed.

✅ 4 resolved
Bug: Placement type mismatch: Ant Design values vs 4 cardinal directions

📄 openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:136 📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormItemLabel/FormItemLabel.tsx:25
The placement prop is cast as TooltipProps['placement'] (Ant Design), which includes compound values like 'top-start', 'topLeft', 'bottomRight', etc. But FormItemLabel only accepts 'top' | 'bottom' | 'left' | 'right'.

This is already used in production: AddDomainForm.component.tsx sets tooltipPlacement: 'top-start'. If a field using FormItemLabel passes such a compound placement, the core Tooltip component will receive an invalid value.

The as cast bypasses TypeScript's type checking, hiding this incompatibility at compile time. Consider either:

  • Widening FormItemLabel's placement type to match what the core Tooltip actually supports
  • Adding runtime normalization (e.g., mapping 'topLeft''top')
Quality: Copyright year should be 2025 or 2026, not 2024

📄 openmetadata-ui/src/main/resources/ui/src/components/common/FormItemLabel/FormItemLabel.tsx:2
The new FormItemLabel.tsx file has Copyright 2024 Collate in the header, but this is a newly created file in 2026. The copyright year should reflect when the file was created.

Bug: valuePropName="checked" is incompatible with Toggle's isSelected

📄 openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:467
The Form.Item wrapping the Toggle component uses valuePropName="checked", but the Toggle component (from React Aria) uses isSelected as its controlled value prop, not checked.

When Ant Design Form manages this field's state (e.g., via form.setFieldsValue(), initialValues, or form.resetFields()), it injects the form value as a prop named checked onto the child component. Since Toggle doesn't recognize checked, form-controlled state won't bind to the Toggle's visual state.

This means:

  1. form.setFieldsValue({ fieldName: true }) won't update the Toggle visually
  2. form.resetFields() won't reset the Toggle to its initial value
  3. The Toggle may appear stuck on its initial state despite form state changes

The fix is to change valuePropName to "isSelected" so Ant Design passes the form value as isSelected, which Toggle recognizes.

Bug: muiLabel as string passes JSX element where Toggle expects string

📄 openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:472
In the UT_SWITCH case, muiLabel is cast as string and passed to Toggle's label prop, but muiLabel is always a React JSX element in practice:

  1. When field.muiLabel is set (e.g., tagFormFields sets it to 'label.disable-tag'), TagsForm.tsx overrides it: muiLabel: <FormItemLabel label={t(field.muiLabel)} /> — a JSX element.
  2. When field.muiLabel is not set (e.g., AddCustomProperty's multiSelectField), the fallback at line 130-139 creates a <FormItemLabel> JSX element.

The Toggle component's label prop is typed as string and renders it inside a <p> tag. Passing a JSX element here will render [object Object] instead of the intended label text.

Fix options:

  • Pass the raw string label (e.g., field.label) to Toggle instead of the muiLabel JSX wrapper, since Toggle handles its own label rendering.
  • Or change Toggle to accept ReactNode for the label prop if JSX labels are needed.
🤖 Prompt for agents
Code Review: Mui switch and form label UI migration addresses placement type mismatches, copyright year updates, and valuePropName incompatibilities with Toggle components. Consider tightening prop spreading in the UT_SWITCH case to avoid unrecognized props, and consolidating the duplicated @openmetadata/ui-core-components mock across test files.

1. 💡 Bug: Unrecognized props spread onto Toggle via switchRest
   Files: openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:463, openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/tagFormFields.tsx:178

   In the `UT_SWITCH` case, `props` is cast as `SwitchProps` and only `disabled` and `onChange` are destructured out. The remaining `switchRest` (which may include `checked`, `label`, `size`, and any extra keys like `inputProps` or `initialValue` from callers like `getDisabledField`) is spread onto the `Toggle` component.
   
   Current callers pass props like `inputProps`, `initialValue`, `data-testid`, and `className` which are not valid `Toggle` props. While `data-testid` and `className` pass through harmlessly as HTML attributes, `inputProps` and `initialValue` are unrecognized and may trigger React warnings about unknown DOM attributes.
   
   Additionally, `{...switchRest}` comes after the explicit `label` prop, so if any caller includes `label` in their props, it would silently override the `muiLabel`-derived label.
   
   Consider destructuring all known props explicitly and only spreading validated rest props, similar to how other field types in this file handle their props.

   Suggested fix:
   case FieldTypes.UT_SWITCH: {
     const { disabled, onChange, size, ...rest } = props as SwitchProps;
   
     return (
       <Form.Item {...formProps} valuePropName="isSelected">
         <Toggle
           isDisabled={disabled}
           label={muiLabel as string}
           size={size}
           onChange={onChange}
         />
       </Form.Item>
     );
   }

2. 💡 Quality: Duplicated `@openmetadata/ui-core-components` mock across 7+ test files
   Files: openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:44, openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx:21, openmetadata-ui/src/main/resources/ui/src/components/common/FormItemLabel/FormItemLabel.test.tsx:20, openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.test.tsx:95

   The identical `jest.mock('@openmetadata/ui-core-components', ...)` block (with Toggle, Tooltip, TooltipTrigger, Badge, and createMuiTheme mocks) is copy-pasted across at least 7 test files:
   
   - TagsPage.test.tsx
   - TagsForm.test.tsx
   - FormItemLabel.test.tsx
   - TestSuitePipelineTab.test.tsx
   - EditTestCaseModal.test.tsx
   - EditTestCaseModalV1.test.tsx
   - TestCaseForm.test.tsx
   - StyleModal.test.tsx
   - TestDefinitionForm.test.tsx
   
   This will become a maintenance burden — any API change to these components requires updating every copy. Consider extracting this into a shared mock file (e.g., `__mocks__/@openmetadata/ui-core-components.tsx` or a shared test utility) and importing it where needed.

Rules 🎸 1 action taken

Gitar Rules

🎸 Flaky Test Retry: Playwright CI job 65886837813 (playwright-ci-postgresql run 3/6) failed with flaky test patterns (timeouts, infrastructure issues unrelated to PR changes); job was re-triggered successfully.

1 rule not applicable. Show all rules by commenting gitar display:verbose.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 5, 2026

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.

1 participant