Skip to content

Mui switch, form label UI migration#26073

Closed
Rohit0301 wants to merge 21 commits intomainfrom
mui-switch-autocomplete-ui-migration
Closed

Mui switch, form label UI migration#26073
Rohit0301 wants to merge 21 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.97% (57337/86906) 45.59% (30226/66289) 48.47% (9083/18738)

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 11, 2026

🔍 CI failure analysis for 36ffe57: 4 test failures in TagsForm component caused by missing Grid component reference after form component refactoring; Grid.Item is undefined when tests attempt to render the form layout.

Overview

Analysis of 3 CI logs reveals a single, high-confidence PR-related test failure pattern. The refactored form components have introduced a breaking change affecting the TagsForm component rendering tests.

Failures

Grid Component Missing in TagsForm Tests (confidence: high)

  • Type: test
  • Affected jobs: 66572880333 (ui-coverage-tests)
  • Related to PR: yes
  • Root cause: PR refactored form components, moving FormItemLabel from Form.tsx to a standalone directory and removing MUIFormItemLabel and MUISwitch components. During this reorganization, the Grid component import or its export chain was broken. TagsForm.tsx attempts to access Grid.Item at line 294 but Grid is undefined.
  • Suggested fix:
    1. Verify TagsForm.tsx has correct Grid import (from antd)
    2. Check that form component index.ts files (e.g., FormItemLabel/index.ts) properly export Grid if it was previously exported from those modules
    3. Add missing Grid import to TagsForm.tsx if not present
    4. Ensure all intermediate form utility files (formUtils.tsx, etc.) properly re-export dependencies

Test Details

Four TagsForm tests fail with identical error:

  • "Form component should render properly"
  • "Form component should render name and displayName fields"
  • "Form component should render Mutually Exclusive field when showMutuallyExclusive is true"
  • "Form component should not render Mutually Exclusive field when showMutuallyExclusive is false"

All fail at line 294 of TagsForm.tsx: <Grid.Item span={12}> with TypeError: Cannot read properties of undefined (reading 'Item')

Summary

  • PR-related failures: 4 failing tests in TagsForm.component due to undefined Grid component reference
  • Infrastructure/flaky failures: None detected
  • Recommended action: Fix the Grid component import chain in form-related files. The issue stems from incomplete refactoring of form components during the MUI migration. Verify that all component reorganization maintained proper imports and exports, particularly for the Grid layout component used by TagsForm.
Code Review 👍 Approved with suggestions 5 resolved / 7 findings

Mui switch and form label UI migration replaces Ant Design components with Material-UI equivalents, resolving placement type mismatches, copyright dates, and prop compatibility issues. Consider consolidating duplicated @openmetadata/ui-core-components mocks across test files and review unrecognized props spreading onto Toggle via switchRest.

💡 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 📄 openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:465 📄 openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:474

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 📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx:14 📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:43 📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:334

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.

✅ 5 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: TagsPage.test.tsx has two jest.mock for same module

📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:43 📄 openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:423
There are two jest.mock('@openmetadata/ui-core-components', ...) calls — a new one at line 43 and an existing one at line 423. Jest hoists all jest.mock calls, and when the same module is mocked twice the last declaration wins. The first mock block (line 43, newly added in this PR) is entirely dead code and will never take effect. The two implementations diverge (e.g., different Toggle and Tooltip behavior). They should be merged into a single mock.

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 replaces Ant Design components with Material-UI equivalents, resolving placement type mismatches, copyright dates, and prop compatibility issues. Consider consolidating duplicated `@openmetadata/ui-core-components` mocks across test files and review unrecognized props spreading onto Toggle via switchRest.

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, openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:465, openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx:474

   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, openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx:14, openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:43, openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.test.tsx:334

   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.

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

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