Conversation
openmetadata-ui/src/main/resources/ui/src/components/common/FormItemLabel/FormItemLabel.tsx
Outdated
Show resolved
Hide resolved
| MOCK_TAGS_CATEGORY, | ||
| } from './TagsPage.mock'; | ||
|
|
||
| jest.mock('@openmetadata/ui-core-components', () => { |
There was a problem hiding this comment.
💡 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
🔍 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.IssueTwo 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) Root CauseAll 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:
Tests failing (job 6/6):
Tests failing (job 3/6):
Why Unrelated to This PRThis PR modifies:
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 findingsMui 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 Current callers pass props like Additionally, 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💡 Quality: Duplicated
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
|



Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
FormItemLabelimplementations and replacedMUISwitchwith core components (Toggle,Badge,Tooltip) from@openmetadata/ui-core-components, eliminating dual Ant Design/Material-UI implementationsMUISwitchwrapper, changedFieldTypes.SWITCH_MUItoFieldTypes.UT_SWITCH, updated form rendering to useTogglewithvaluePropName="isSelected"FormItemLabelto dedicated directory using Tailwind CSS styling (tw:classes) and@untitledui/icons, standardized test IDs frommui-*toform-item-*Form/MUIFormItemLabelpaths to newFormItemLabellocation with consistent tooltip placement type (@react-types/overlaysPlacement)This will update automatically on new commits.