From 81fcec41a6aee8e765f186ed39df5112dbd53ef7 Mon Sep 17 00:00:00 2001 From: Rohit0301 Date: Mon, 6 Jul 2026 17:38:18 +0530 Subject: [PATCH 1/4] refactor(ui): migrate TagsForm from antd Form to React Hook Form + Autocomplete Replace the antd Form/FormInstance pattern with react-hook-form (UseFormReturn) and swap MUI owner/domain pickers for Autocomplete-based USER_TEAM_SELECT_INPUT and DOMAIN_SELECT fields, following the AddDomainForm pattern exactly. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ClassificationFormDrawer.test.tsx | 16 +- .../TagsPage/ClassificationFormDrawer.tsx | 10 +- .../src/pages/TagsPage/TagFormDrawer.test.tsx | 16 +- .../ui/src/pages/TagsPage/TagFormDrawer.tsx | 10 +- .../ui/src/pages/TagsPage/TagsForm.test.tsx | 20 +- .../ui/src/pages/TagsPage/TagsForm.tsx | 549 ++++++++++++------ .../src/pages/TagsPage/TagsPage.interface.ts | 40 +- .../ui/src/pages/TagsPage/TagsPage.tsx | 28 +- .../src/pages/TagsPage/tagFormFields.test.tsx | 419 +++---------- .../ui/src/pages/TagsPage/tagFormFields.tsx | 153 ++--- 10 files changed, 596 insertions(+), 665 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.test.tsx index c1ff0ed56c43..c11a9eef821d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.test.tsx @@ -12,8 +12,9 @@ */ import { fireEvent, render, screen } from '@testing-library/react'; -import { FormInstance } from 'antd'; +import { UseFormReturn } from 'react-hook-form'; import ClassificationFormDrawer from './ClassificationFormDrawer'; +import { TagFormValues } from './TagsPage.interface'; jest.mock('./TagsForm', () => { return jest.fn(() =>
Tags Form
); @@ -93,12 +94,11 @@ jest.mock('@openmetadata/ui-core-components', () => ({ })); const mockForm = { - submit: jest.fn(), - resetFields: jest.fn(), - getFieldsValue: jest.fn(), - setFieldsValue: jest.fn(), - validateFields: jest.fn(), -} as unknown as FormInstance; + control: {}, + handleSubmit: jest.fn(), + reset: jest.fn(), + formState: { isSubmitSuccessful: false }, +} as unknown as UseFormReturn; describe('ClassificationFormDrawer', () => { const mockOnClose = jest.fn(); @@ -106,7 +106,7 @@ describe('ClassificationFormDrawer', () => { const defaultProps = { open: true, - formRef: mockForm, + form: mockForm, classifications: [], isTier: false, isLoading: false, diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.tsx index 6e4d0ecbedc2..f88ed04d11ef 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/ClassificationFormDrawer.tsx @@ -16,14 +16,14 @@ import { SlideoutMenu, Typography, } from '@openmetadata/ui-core-components'; -import { FC, useCallback } from 'react'; +import { FC, useCallback, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import TagsForm from './TagsForm'; import { ClassificationFormDrawerProps } from './TagsPage.interface'; const ClassificationFormDrawer: FC = ({ open, - formRef, + form, classifications, isTier, isLoading, @@ -31,6 +31,7 @@ const ClassificationFormDrawer: FC = ({ onSubmit, }) => { const { t } = useTranslation(); + const submitRef = useRef<() => void>(() => void 0); const handleOpenChange = useCallback( (isOpen: boolean) => { @@ -61,9 +62,10 @@ const ClassificationFormDrawer: FC = ({ isClassification showMutuallyExclusive data={classifications} - formRef={formRef} + form={form} isEditing={false} isTier={isTier} + submitRef={submitRef} onSubmit={onSubmit} /> @@ -81,7 +83,7 @@ const ClassificationFormDrawer: FC = ({ data-testid="save-button" isDisabled={isLoading} isLoading={isLoading} - onClick={() => formRef.submit()}> + onClick={() => submitRef.current()}> {t('label.save')} diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.test.tsx index 3a1698e1dc27..92bf914711a7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.test.tsx @@ -12,8 +12,9 @@ */ import { fireEvent, render, screen } from '@testing-library/react'; -import { FormInstance } from 'antd'; +import { UseFormReturn } from 'react-hook-form'; import TagFormDrawer from './TagFormDrawer'; +import { TagFormValues } from './TagsPage.interface'; jest.mock('./TagsForm', () => { return jest.fn(() =>
Tags Form
); @@ -93,12 +94,11 @@ jest.mock('@openmetadata/ui-core-components', () => ({ })); const mockForm = { - submit: jest.fn(), - resetFields: jest.fn(), - getFieldsValue: jest.fn(), - setFieldsValue: jest.fn(), - validateFields: jest.fn(), -} as unknown as FormInstance; + control: {}, + handleSubmit: jest.fn(), + reset: jest.fn(), + formState: { isSubmitSuccessful: false }, +} as unknown as UseFormReturn; describe('TagFormDrawer', () => { const mockOnClose = jest.fn(); @@ -106,7 +106,7 @@ describe('TagFormDrawer', () => { const defaultProps = { open: true, - formRef: mockForm, + form: mockForm, isTier: false, isLoading: false, permissions: { diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.tsx index bd06989465d9..a5b7dc63f382 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagFormDrawer.tsx @@ -17,7 +17,7 @@ import { Typography, } from '@openmetadata/ui-core-components'; import { isUndefined } from 'lodash'; -import { FC, useCallback } from 'react'; +import { FC, useCallback, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import { ProviderType } from '../../generated/api/classification/createTag'; import TagsForm from './TagsForm'; @@ -26,7 +26,7 @@ import { TagFormDrawerProps } from './TagsPage.interface'; const TagFormDrawer: FC = ({ open, editTag, - formRef, + form, isTier, isLoading, permissions, @@ -35,6 +35,7 @@ const TagFormDrawer: FC = ({ onSubmit, }) => { const { t } = useTranslation(); + const submitRef = useRef<() => void>(() => void 0); const handleOpenChange = useCallback( (isOpen: boolean) => { @@ -62,13 +63,14 @@ const TagFormDrawer: FC = ({ @@ -86,7 +88,7 @@ const TagFormDrawer: FC = ({ data-testid="save-button" isDisabled={isLoading} isLoading={isLoading} - onClick={() => formRef.submit()}> + onClick={() => submitRef.current()}> {t('label.save')} diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx index 4e7b301829b0..8761de5b1a75 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx @@ -12,11 +12,10 @@ */ import { render, screen } from '@testing-library/react'; -import { Form } from 'antd'; +import { useForm } from 'react-hook-form'; import { DEFAULT_FORM_VALUE } from '../../constants/Tags.constant'; -import { Classification } from '../../generated/entity/classification/classification'; -import { Tag } from '../../generated/entity/classification/tag'; import TagsForm from './TagsForm'; +import { TagFormValues } from './TagsPage.interface'; jest.mock('@openmetadata/ui-core-components', () => { const GridItem = ({ children }: { children: React.ReactNode }) => ( @@ -122,12 +121,12 @@ const TestWrapper = ({ showMutuallyExclusive?: boolean; isClassification?: boolean; }) => { - const [formRef] = Form.useForm(); + const form = useForm(); return ( { }); it('Form component should render Mutually Exclusive field when showMutuallyExclusive is true', async () => { - const { container } = render( - - ); + render(); - // Check for the Form.Item with id that contains mutuallyExclusive - const mutuallyExclusiveFormItem = container.querySelector( - '#tags_mutuallyExclusive' + const mutuallyExclusiveButton = await screen.findByTestId( + 'mutually-exclusive-button' ); - expect(mutuallyExclusiveFormItem).toBeInTheDocument(); + expect(mutuallyExclusiveButton).toBeInTheDocument(); }); it('Form component should not render Mutually Exclusive field when showMutuallyExclusive is false', async () => { diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx index fc6f968648d6..0d057ff027be 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx @@ -11,44 +11,83 @@ * limitations under the License. */ -import { Grid } from '@openmetadata/ui-core-components'; -import { Form } from 'antd'; -import { castArray } from 'lodash'; -import { Suspense, useEffect, useMemo } from 'react'; +import { + Avatar, + Box, + FieldProp, + FormField, + FormItemLabel, + Grid, + HintText, + HookForm, + getField, +} from '@openmetadata/ui-core-components'; +import { Users01 } from '@untitledui/icons'; +import { debounce } from 'lodash'; +import { Suspense, useCallback, useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; +import { useWatch } from 'react-hook-form'; import { EntityAttachmentProvider } from '../../components/common/EntityDescription/EntityAttachmentProvider/EntityAttachmentProvider'; -import MUIFormItemLabel from '../../components/common/MUIFormItemLabel/MUIFormItemLabel'; -import { VALIDATION_MESSAGES } from '../../constants/constants'; import { - DEFAULT_FORM_VALUE, - TAG_NAME_VALIDATION_RULES, -} from '../../constants/Tags.constant'; + AVAILABLE_ICONS, + DEFAULT_TAG_ICON, +} from '../../components/common/IconPicker'; +import RichTextEditor from '../../components/common/RichTextEditor/RichTextEditor'; +import { PAGE_SIZE_MEDIUM } from '../../constants/constants'; import { EntityType } from '../../enums/entity.enum'; +import { SearchIndex } from '../../enums/search.enum'; import { CreateClassification } from '../../generated/api/classification/createClassification'; import { CreateTag } from '../../generated/api/classification/createTag'; import { Classification } from '../../generated/entity/classification/classification'; import { Tag } from '../../generated/entity/classification/tag'; import { EntityReference } from '../../generated/entity/type'; import { useEntityRules } from '../../hooks/useEntityRules'; -import { FieldProp } from '../../interface/FormUtils.interface'; -import { generateFormFields, getField } from '../../utils/formUtils'; +import { searchDomains } from '../../rest/domainAPI'; +import { searchQuery } from '../../rest/searchAPI'; +import { formatTeamsResponse } from '../../utils/APIUtils'; +import { getRandomColor } from '../../utils/ColorUtils'; +import { getEntityName } from '../../utils/EntityNameUtils'; +import { getEntityReferenceListFromEntities } from '../../utils/EntityReferenceUtils'; +import { getTermQuery } from '../../utils/SearchPureUtils'; import tagClassBase from '../../utils/TagClassBase'; import { COLOR_FIELD, - getDescriptionField, getDisabledField, - getDisplayNameField, getDomainField, getIconField, getMutuallyExclusiveField, getNameField, + getDisplayNameField, getOwnerField, } from './tagFormFields'; import './TagsForm.less'; -import { RenameFormProps } from './TagsPage.interface'; +import { + RenameFormProps, + TAG_FORM_DEFAULTS, + TagFormSelectItem, + TagFormValues, +} from './TagsPage.interface'; + +const mapEntityReferenceToSelectItem = ( + ref: EntityReference +): TagFormSelectItem => ({ + id: ref.id, + label: getEntityName(ref), + supportingText: ref.fullyQualifiedName ?? ref.type, + value: ref, +}); + +const convertToTagFormValues = ( + entity: Classification | Tag +): Partial => ({ + ...entity, + owners: (entity.owners ?? []).map(mapEntityReferenceToSelectItem), + domains: (entity.domains ?? []).map(mapEntityReferenceToSelectItem), +}); const TagsForm = ({ - formRef, + form, + submitRef, initialValues, onSubmit, showMutuallyExclusive = false, @@ -60,22 +99,35 @@ const TagsForm = ({ }: RenameFormProps) => { const { t } = useTranslation(); const { entityRules } = useEntityRules(EntityType.CLASSIFICATION); - const selectedColor = Form.useWatch(['style', 'color'], formRef); - const selectedDomain = Form.useWatch( - 'domains', - formRef - ); - const selectedOwners = Form.useWatch< - EntityReference | EntityReference[] | undefined - >('owners', formRef); - const isMutuallyExclusive = Form.useWatch( - 'mutuallyExclusive', - formRef + const [userTeamOptions, setUserTeamOptions] = useState( + [] ); + const [domainOptions, setDomainOptions] = useState([]); + const [descriptionEditorKey, setDescriptionEditorKey] = useState(0); + + const selectedColor = useWatch({ + control: form.control, + name: 'style.color', + }); + + const isMutuallyExclusive = useWatch({ + control: form.control, + name: 'mutuallyExclusive', + }); useEffect(() => { - formRef?.setFieldsValue(initialValues); - }, [initialValues, formRef]); + if (initialValues) { + form.reset(convertToTagFormValues(initialValues) as TagFormValues); + } else { + form.reset(TAG_FORM_DEFAULTS); + } + }, [initialValues, form]); + + useEffect(() => { + if (form.formState.isSubmitSuccessful) { + setDescriptionEditorKey((prev) => prev + 1); + } + }, [form.formState.isSubmitSuccessful]); const disableNameField = useMemo( () => isEditing && isSystemTag, @@ -111,48 +163,196 @@ const TagsForm = ({ [isEditing, isClassification, permissions] ); - const iconField = useMemo(() => { - const field = getIconField(selectedColor); + const fetchUserTeamOptions = useCallback(async (searchText = '') => { + try { + const [usersResponse, teamsResponse] = await Promise.all([ + searchQuery({ + pageNumber: 1, + pageSize: PAGE_SIZE_MEDIUM, + query: searchText, + queryFilter: getTermQuery({ isBot: 'false' }), + searchIndex: SearchIndex.USER, + sortField: 'displayName.keyword', + sortOrder: 'asc', + }), + searchQuery({ + pageNumber: 1, + pageSize: PAGE_SIZE_MEDIUM, + query: searchText, + queryFilter: getTermQuery({}, 'must', undefined, { + matchTerms: { teamType: 'Group' }, + }), + searchIndex: SearchIndex.TEAM, + sortField: 'displayName.keyword', + sortOrder: 'asc', + }), + ]); + + const userOptions = usersResponse.hits.hits.map((hit) => { + const source = hit._source; + const name = getEntityName(source); + const { color, backgroundColor, character } = getRandomColor( + source.displayName ?? source.name ?? '' + ); + + return { + id: source.id, + label: name, + supportingText: source.fullyQualifiedName ?? EntityType.USER, + icon: ( + + ), + value: { + id: source.id, + type: EntityType.USER, + name: source.name, + displayName: source.displayName, + fullyQualifiedName: source.fullyQualifiedName, + } as EntityReference, + }; + }); - return { - ...field, - muiLabel: , - props: { - ...field.props, - placeholder: t(field.placeholder), - }, - }; - }, [t, selectedColor]); + const teams = getEntityReferenceListFromEntities( + formatTeamsResponse(teamsResponse.hits.hits), + EntityType.TEAM + ); + + setUserTeamOptions([ + ...userOptions, + ...teams.map((reference) => ({ + ...mapEntityReferenceToSelectItem(reference), + icon: , + })), + ]); + } catch { + setUserTeamOptions([]); + } + }, []); - const colorField = useMemo( - () => ({ - ...COLOR_FIELD, - muiLabel: , - }), - [t] + const fetchDomainOptions = useCallback(async (searchText = '') => { + try { + const domains = await searchDomains(searchText, 1); + const nextOptions = domains.map((domain) => + mapEntityReferenceToSelectItem({ + displayName: domain.displayName, + fullyQualifiedName: domain.fullyQualifiedName, + id: domain.id, + name: domain.name, + type: EntityType.DOMAIN, + }) + ); + setDomainOptions(nextOptions); + } catch { + setDomainOptions([]); + } + }, []); + + const handleUserTeamFocus = useCallback(() => { + void fetchUserTeamOptions(); + }, [fetchUserTeamOptions]); + + const handleDomainFocus = useCallback(() => { + void fetchDomainOptions(); + }, [fetchDomainOptions]); + + const debouncedUserTeamSearch = useMemo( + () => + debounce( + (searchText: string) => void fetchUserTeamOptions(searchText), + 250 + ), + [fetchUserTeamOptions] + ); + + const debouncedDomainSearch = useMemo( + () => + debounce( + (searchText: string) => void fetchDomainOptions(searchText), + 250 + ), + [fetchDomainOptions] + ); + + useEffect( + () => () => { + debouncedUserTeamSearch.cancel(); + debouncedDomainSearch.cancel(); + }, + [debouncedUserTeamSearch, debouncedDomainSearch] ); + const iconOptions = useMemo( + () => + [ + DEFAULT_TAG_ICON, + ...AVAILABLE_ICONS.filter((icon) => icon.name !== DEFAULT_TAG_ICON.name), + ].map((icon) => ({ + id: icon.name, + icon: icon.component, + label: icon.name, + value: icon.name, + })), + [] + ); + + const handleSave = useCallback( + async (formData: TagFormValues) => { + const owners = (formData.owners ?? []).map( + (item) => item.value as EntityReference + ); + const domainItems = formData.domains ?? []; + + let domainsData; + if (domainItems.length > 0) { + if (isEditing) { + domainsData = domainItems.map( + (item) => item.value as EntityReference + ); + } else { + domainsData = domainItems + .map((item) => { + const ref = item.value as EntityReference; + + return ref.fullyQualifiedName ?? ref.name; + }) + .filter(Boolean); + } + } + + const submitData = { + ...formData, + owners: owners.length ? owners : undefined, + domains: domainsData, + } as CreateClassification | CreateTag; + + try { + await onSubmit(submitData); + form.reset(TAG_FORM_DEFAULTS); + } catch { + // Parent will handle the error + } + }, + [onSubmit, isEditing, form] + ); + + useEffect(() => { + if (submitRef) { + submitRef.current = () => void form.handleSubmit(handleSave)(); + } + }, [form, handleSave, submitRef]); + const nameField = useMemo(() => { const field = getNameField(disableNameField || false); return { ...field, - muiLabel: t(field.muiLabel), - placeholder: t(field.placeholder), - rules: TAG_NAME_VALIDATION_RULES.map((rule) => ({ - ...rule, - message: rule.message - ? t(rule.message, { - ...rule.messageData, - field: rule.messageData?.field - ? t(rule.messageData.field) - : undefined, - entity: rule.messageData?.entity - ? t(rule.messageData.entity) - : undefined, - }) - : undefined, - })), + label: t(field.label as string), + placeholder: t(field.placeholder ?? ''), }; }, [t, disableNameField]); @@ -161,83 +361,93 @@ const TagsForm = ({ return { ...field, - muiLabel: t(field.muiLabel), - placeholder: t(field.placeholder), + label: t(field.label as string), + placeholder: t(field.placeholder ?? ''), }; }, [t, disableDisplayNameField]); - const ownerField = useMemo(() => { - const field = getOwnerField({ - canAddMultipleUserOwners: entityRules.canAddMultipleUserOwners, - canAddMultipleTeamOwner: entityRules.canAddMultipleTeamOwner, - }); + const iconField = useMemo( + () => ({ + ...getIconField(selectedColor, iconOptions as TagFormSelectItem[]), + label: t('label.icon'), + }), + [t, selectedColor, iconOptions] + ); - return { - ...field, - muiLabel: t(field.muiLabel), - }; - }, [ - t, - entityRules.canAddMultipleUserOwners, - entityRules.canAddMultipleTeamOwner, - ]); - - const domainField = useMemo(() => { - const field = getDomainField({ - canAddMultipleDomains: entityRules.canAddMultipleDomains, + const colorField = useMemo( + () => ({ + ...COLOR_FIELD, + label: t('label.color'), + }), + [t] + ); + + const ownerField = useMemo( + (): FieldProp => ({ + ...getOwnerField({ + multiple: + entityRules.canAddMultipleUserOwners || + entityRules.canAddMultipleTeamOwner, + options: userTeamOptions, + onFocus: handleUserTeamFocus, + onSearchChange: (searchText: string) => + debouncedUserTeamSearch(searchText), + }), + label: t('label.owner-plural'), + }), + [ + t, + entityRules.canAddMultipleUserOwners, + entityRules.canAddMultipleTeamOwner, + userTeamOptions, + handleUserTeamFocus, + debouncedUserTeamSearch, + ] + ); + + const domainField = useMemo( + (): FieldProp => ({ + ...getDomainField({ + canAddMultipleDomains: entityRules.canAddMultipleDomains, + options: domainOptions, + onFocus: handleDomainFocus, + onSearchChange: (searchText: string) => + debouncedDomainSearch(searchText), + }), + label: t('label.domain-plural'), + }), + [ + t, + entityRules.canAddMultipleDomains, + domainOptions, + handleDomainFocus, + debouncedDomainSearch, + ] + ); + + const disabledField = useMemo(() => { + const field = getDisabledField({ + initialValue: (initialValues as Tag)?.disabled ?? false, + disabled: disableDisabledField, }); return { ...field, - muiLabel: t(field.muiLabel), + label: t(field.label as string), }; - }, [entityRules.canAddMultipleDomains, t]); - - const formFields: FieldProp[] = useMemo(() => { - const descriptionField = getDescriptionField({ - initialValue: initialValues?.description ?? '', - readonly: disableDescriptionField, - }); - - const fields: FieldProp[] = [ - { - ...descriptionField, - label: , - }, - ]; - - if (isSystemTag && !isTier) { - const disabledField = getDisabledField({ - initialValue: initialValues?.disabled ?? false, - disabled: disableDisabledField, - }); - - fields.push({ - ...disabledField, - label: t(disabledField.label), - }); - } - - return fields; - }, [ - t, - initialValues?.description, - initialValues?.disabled, - disableDescriptionField, - disableDisabledField, - isSystemTag, - isTier, - ]); + }, [t, initialValues, disableDisabledField]); const mutuallyExclusiveField = useMemo(() => { const field = getMutuallyExclusiveField({ disabled: disableMutuallyExclusiveField, - showHelperText: Boolean(isMutuallyExclusive), + helperText: isMutuallyExclusive + ? t('message.mutually-exclusive-alert-info') + : undefined, }); return { ...field, - label: t(field.label), + label: t(field.label as string), }; }, [t, disableMutuallyExclusiveField, isMutuallyExclusive]); @@ -247,76 +457,81 @@ const TagsForm = ({ [isClassification] ); - const handleSave = async (data: Classification | Tag | undefined) => { - const domains = castArray(selectedDomain).filter(Boolean); - const owners = castArray(selectedOwners).filter(Boolean); - - try { - let domainsData; - if (domains?.length) { - if (isEditing) { - domainsData = domains; - } else { - domainsData = domains - .map((domain) => domain.fullyQualifiedName ?? domain.name) - .filter(Boolean); - } - } - - const submitData = { - ...data, - owners: owners?.length ? owners : undefined, - domains: domainsData, - } as CreateClassification | CreateTag; - await onSubmit(submitData); - formRef.setFieldsValue(DEFAULT_FORM_VALUE); - } catch { - // Parent will handle the error - } - }; - return ( -
+ form={form} + onSubmit={form.handleSubmit(handleSave)}> {getField(nameField)} {getField(displayNameField)} {!isClassification && ( - - {getField(iconField)} - {getField(colorField)} - + +
+ {getField(iconField)} +
+
+ {getField(colorField)} +
+ )} - {generateFormFields(formFields)} -
- {showMutuallyExclusive && getField(mutuallyExclusiveField)} -
+ + {({ field, fieldState }) => ( + + + + {fieldState.error?.message && ( + {fieldState.error.message} + )} + + )} + + + {isSystemTag && !isTier && ( +
{getField(disabledField)}
+ )} + + {showMutuallyExclusive && ( +
{getField(mutuallyExclusiveField)}
+ )} {getField(ownerField)} {getField(domainField)} - {/* Auto Classification fields */} {autoClassificationComponent && ( {autoClassificationComponent} )} -
+ ); }; diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.interface.ts b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.interface.ts index 11f2212edc97..acb7fe73dc24 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.interface.ts @@ -11,12 +11,15 @@ * limitations under the License. */ -import { FormInstance } from 'antd'; +import { FormSelectItem } from '@openmetadata/ui-core-components'; import { LoadingState } from 'Models'; +import { MutableRefObject } from 'react'; +import { UseFormReturn } from 'react-hook-form'; import { CreateClassification } from '../../generated/api/classification/createClassification'; import { CreateTag } from '../../generated/api/classification/createTag'; import { Classification } from '../../generated/entity/classification/classification'; import { Tag } from '../../generated/entity/classification/tag'; +import { EntityReference } from '../../generated/entity/type'; export type DeleteTagDetailsType = { id: string; @@ -31,8 +34,37 @@ export type DeleteTagsType = { state: boolean; }; +export interface TagFormSelectItem extends FormSelectItem { + value: EntityReference | string; +} + +export interface TagFormValues { + id?: string; + name: string; + displayName?: string; + description?: string; + style?: { + color?: string; + iconURL?: string; + }; + disabled?: boolean; + mutuallyExclusive?: boolean; + owners?: TagFormSelectItem[]; + domains?: TagFormSelectItem[]; +} + +export const TAG_FORM_DEFAULTS: TagFormValues = { + id: '', + name: '', + displayName: '', + description: '', + owners: [], + domains: [], +}; + export interface RenameFormProps { - formRef: FormInstance; + form: UseFormReturn; + submitRef?: MutableRefObject<() => void>; isEditing: boolean; isTier: boolean; initialValues?: Classification | Tag; @@ -51,7 +83,7 @@ export interface RenameFormProps { export interface ClassificationFormDrawerProps { open: boolean; - formRef: FormInstance; + form: UseFormReturn; classifications: Classification[]; isTier: boolean; isLoading: boolean; @@ -62,7 +94,7 @@ export interface ClassificationFormDrawerProps { export interface TagFormDrawerProps { open: boolean; editTag?: Tag; - formRef: FormInstance; + form: UseFormReturn; isTier: boolean; isLoading: boolean; permissions: { diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.tsx index 2afe48a48c39..f377b5deef3f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsPage.tsx @@ -12,7 +12,7 @@ */ import { Badge, Button, Typography } from '@openmetadata/ui-core-components'; -import { useForm } from 'antd/lib/form/Form'; +import { useForm } from 'react-hook-form'; import { AxiosError } from 'axios'; import classNames from 'classnames'; import { compare } from 'fast-json-patch'; @@ -68,15 +68,21 @@ import tagClassBase from '../../utils/TagClassBase'; import { showErrorToast } from '../../utils/ToastUtils'; import ClassificationFormDrawer from './ClassificationFormDrawer'; import TagFormDrawer from './TagFormDrawer'; -import { DeleteTagsType } from './TagsPage.interface'; +import { + DeleteTagsType, + TAG_FORM_DEFAULTS, + TagFormValues, +} from './TagsPage.interface'; const TagsPage = () => { const { getEntityPermission, permissions } = usePermissionProvider(); const { t } = useTranslation(); const navigate = useNavigate(); const { fqn: tagCategoryName } = useFqn(); - const [tagForm] = useForm(); - const [classificationForm] = useForm(); + const tagForm = useForm({ defaultValues: TAG_FORM_DEFAULTS }); + const classificationForm = useForm({ + defaultValues: TAG_FORM_DEFAULTS, + }); const [classifications, setClassifications] = useState>( [] ); @@ -597,7 +603,7 @@ const TagsPage = () => { const handleTagDrawerClose = useCallback(() => { setIsTagDrawerOpen(false); - tagForm.resetFields(); + tagForm.reset(); setEditTag(undefined); }, [tagForm]); @@ -607,12 +613,12 @@ const TagsPage = () => { const handleClassificationDrawerClose = useCallback(() => { setIsClassificationDrawerOpen(false); - classificationForm.resetFields(); + classificationForm.reset(); }, [classificationForm]); const handleClassificationDrawerOpen = useCallback(() => { setIsClassificationDrawerOpen(true); - classificationForm.resetFields(); + classificationForm.reset(); }, [classificationForm]); const handleTagFormSubmit = useCallback( @@ -651,7 +657,7 @@ const TagsPage = () => { const handleAddNewTagClick = useCallback(() => { setEditTag(undefined); - tagForm.resetFields(); + tagForm.reset(); handleTagDrawerOpen(); }, [handleTagDrawerOpen, tagForm]); @@ -671,7 +677,7 @@ const TagsPage = () => { iconLeading={} size="sm" onClick={() => { - classificationForm.resetFields(); + classificationForm.reset(); handleClassificationDrawerOpen(); }}> @@ -807,7 +813,7 @@ const TagsPage = () => { { ({ - iconTooltipDataRender: jest.fn(() => 'mocked-tooltip'), -})); +const noopFn = jest.fn(); describe('tagFormFields', () => { describe('getIconField', () => { - it('should return icon field configuration with default values', () => { + it('should return icon field with ICON_PICKER type', () => { const result = getIconField(); - expect(result).toEqual({ - name: ['style', 'iconURL'], - id: 'root/style/iconURL', - label: 'label.icon', - muiLabel: 'label.icon', - required: false, - type: FieldTypes.ICON_PICKER_MUI, - helperText: 'mocked-tooltip', - placeholder: 'label.icon-url', - formItemLayout: FormItemLayout.HORIZONTAL, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, - props: { - 'data-testid': 'icon-picker-btn', - allowUrl: true, - backgroundColor: undefined, - defaultIcon: DEFAULT_TAG_ICON, - customStyles: { - searchBoxWidth: 366, - }, - }, - }); + expect(result.name).toBe('style.iconURL'); + expect(result.type).toBe(FieldTypes.ICON_PICKER); + expect(result.required).toBe(false); }); - it('should return icon field configuration with selected color', () => { + it('should pass backgroundColor from selected color', () => { const selectedColor = '#FF5733'; const result = getIconField(selectedColor); expect(result.props?.backgroundColor).toBe(selectedColor); }); - - it('should have correct formItemProps', () => { - const result = getIconField(); - - expect(result.formItemProps).toEqual({ - valuePropName: 'value', - trigger: 'onChange', - }); - }); - - it('should have correct custom styles', () => { - const result = getIconField(); - - expect(result.props?.customStyles).toEqual({ - searchBoxWidth: 366, - }); - }); }); describe('COLOR_FIELD', () => { - it('should have correct color field configuration', () => { - expect(COLOR_FIELD).toEqual({ - name: ['style', 'color'], - id: 'root/style/color', - label: 'label.color', - muiLabel: 'label.color', - required: false, - type: FieldTypes.COLOR_PICKER_MUI, - formItemLayout: FormItemLayout.HORIZONTAL, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, - }); - }); - - it('should be a constant field', () => { - expect(COLOR_FIELD.type).toBe(FieldTypes.COLOR_PICKER_MUI); + it('should have COLOR_PICKER type and correct name', () => { + expect(COLOR_FIELD.type).toBe(FieldTypes.COLOR_PICKER); + expect(COLOR_FIELD.name).toBe('style.color'); expect(COLOR_FIELD.required).toBe(false); }); }); describe('getNameField', () => { - it('should return name field configuration when disabled is false', () => { - const result = getNameField(false); - - expect(result).toEqual({ - name: 'name', - id: 'root/name', - label: 'label.name', - muiLabel: 'label.name', - required: true, - placeholder: 'label.name', - type: FieldTypes.TEXT_MUI, - props: { - inputProps: { - 'data-testid': 'name', - }, - disabled: false, - }, - formItemProps: { - validateTrigger: ['onChange', 'onBlur'], - }, - }); - }); - - it('should return name field configuration when disabled is true', () => { - const result = getNameField(true); - - expect(result.props?.disabled).toBe(true); - }); - - it('should be a required field', () => { + it('should return TEXT type with data-testid', () => { const result = getNameField(false); + expect(result.type).toBe(FieldTypes.TEXT); + expect(result.name).toBe('name'); expect(result.required).toBe(true); + expect(result.props?.['data-testid']).toBe('name'); }); - it('should have correct validation triggers', () => { - const result = getNameField(false); - - expect(result.formItemProps?.validateTrigger).toEqual([ - 'onChange', - 'onBlur', - ]); + it('should forward disabled prop', () => { + expect(getNameField(true).props?.disabled).toBe(true); + expect(getNameField(false).props?.disabled).toBe(false); }); }); describe('getDisplayNameField', () => { - it('should return display name field configuration when disabled is false', () => { + it('should return TEXT type and be optional', () => { const result = getDisplayNameField(false); - expect(result).toEqual({ - name: 'displayName', - id: 'root/displayName', - label: 'label.display-name', - muiLabel: 'label.display-name', - required: false, - placeholder: 'label.display-name', - type: FieldTypes.TEXT_MUI, - props: { - inputProps: { - 'data-testid': 'displayName', - }, - disabled: false, - }, - }); - }); - - it('should return display name field configuration when disabled is true', () => { - const result = getDisplayNameField(true); - - expect(result.props?.disabled).toBe(true); + expect(result.type).toBe(FieldTypes.TEXT); + expect(result.name).toBe('displayName'); + expect(result.required).toBe(false); }); - it('should be an optional field', () => { - const result = getDisplayNameField(false); - - expect(result.required).toBe(false); + it('should forward disabled prop', () => { + expect(getDisplayNameField(true).props?.disabled).toBe(true); }); }); describe('getOwnerField', () => { - it('should return owner field configuration with multiple users and teams disabled', () => { - const result = getOwnerField({ - canAddMultipleUserOwners: false, - canAddMultipleTeamOwner: false, - }); - - expect(result).toEqual({ - name: 'owners', - id: 'root/owner', - required: false, - label: 'label.owner-plural', - muiLabel: 'label.owner-plural', - type: FieldTypes.USER_TEAM_SELECT_MUI, - props: { - multipleUser: false, - multipleTeam: false, - }, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, - }); - }); - - it('should return owner field configuration with multiple users enabled', () => { - const result = getOwnerField({ - canAddMultipleUserOwners: true, - canAddMultipleTeamOwner: false, - }); - - expect(result.props?.multipleUser).toBe(true); - expect(result.props?.multipleTeam).toBe(false); - }); - - it('should return owner field configuration with multiple teams enabled', () => { + it('should return USER_TEAM_SELECT_INPUT with provided options and callbacks', () => { + const options = [{ id: '1', label: 'Alice', value: 'ref1' }]; const result = getOwnerField({ - canAddMultipleUserOwners: false, - canAddMultipleTeamOwner: true, + multiple: true, + options, + onFocus: noopFn, + onSearchChange: noopFn, }); - expect(result.props?.multipleUser).toBe(false); - expect(result.props?.multipleTeam).toBe(true); - }); - - it('should return owner field configuration with both multiple options enabled', () => { - const result = getOwnerField({ - canAddMultipleUserOwners: true, - canAddMultipleTeamOwner: true, - }); - - expect(result.props?.multipleUser).toBe(true); - expect(result.props?.multipleTeam).toBe(true); + expect(result.type).toBe(FieldTypes.USER_TEAM_SELECT_INPUT); + expect(result.name).toBe('owners'); + expect(result.props?.multiple).toBe(true); + expect(result.props?.options).toBe(options); + expect(result.props?.onFocus).toBe(noopFn); + expect(result.props?.onSearchChange).toBe(noopFn); }); }); describe('getDomainField', () => { - it('should return domain field configuration with multiple domains disabled', () => { - const result = getDomainField({ - canAddMultipleDomains: false, - }); - - expect(result).toEqual({ - name: 'domains', - id: 'root/domains', - required: false, - label: 'label.domain-plural', - muiLabel: 'label.domain-plural', - type: FieldTypes.DOMAIN_SELECT_MUI, - props: { - 'data-testid': 'domain-select', - hasPermission: true, - multiple: false, - }, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, - }); - }); - - it('should return domain field configuration with multiple domains enabled', () => { + it('should return DOMAIN_SELECT with multiple flag from canAddMultipleDomains', () => { const result = getDomainField({ canAddMultipleDomains: true, + options: [], + onFocus: noopFn, + onSearchChange: noopFn, }); + expect(result.type).toBe(FieldTypes.DOMAIN_SELECT); + expect(result.name).toBe('domains'); expect(result.props?.multiple).toBe(true); }); - it('should always have hasPermission set to true', () => { + it('should set multiple to false when canAddMultipleDomains is false', () => { const result = getDomainField({ canAddMultipleDomains: false, + options: [], + onFocus: noopFn, + onSearchChange: noopFn, }); - expect(result.props?.hasPermission).toBe(true); - }); - }); - - describe('getDescriptionField', () => { - it('should return description field configuration with default values', () => { - const initialValue = 'Test description'; - const result = getDescriptionField({ - initialValue, - readonly: false, - }); - - expect(result).toEqual({ - name: 'description', - required: true, - label: 'label.description', - id: 'root/description', - type: FieldTypes.DESCRIPTION, - props: { - 'data-testid': 'description', - initialValue, - readonly: false, - className: 'description-text-area', - }, - formItemProps: { - className: 'description-form-item', - }, - }); - }); - - it('should return description field configuration with readonly true', () => { - const result = getDescriptionField({ - initialValue: '', - readonly: true, - }); - - expect(result.props?.readonly).toBe(true); - }); - - it('should be a required field', () => { - const result = getDescriptionField({ - initialValue: '', - readonly: false, - }); - - expect(result.required).toBe(true); - }); - - it('should preserve initialValue', () => { - const initialValue = 'Custom initial value'; - const result = getDescriptionField({ - initialValue, - readonly: false, - }); - - expect(result.props?.initialValue).toBe(initialValue); + expect(result.props?.multiple).toBe(false); }); }); describe('getDisabledField', () => { - it('should return disabled field configuration with default values', () => { - const result = getDisabledField({ - initialValue: false, - disabled: false, - }); - - expect(result).toEqual({ - name: 'disabled', - required: false, - label: 'label.disable-tag', - id: 'root/disabled', - type: FieldTypes.UT_SWITCH, - formItemLayout: FormItemLayout.HORIZONTAL, - props: { - 'data-testid': 'disabled', - initialValue: false, - isDisabled: false, - }, - }); - }); - - it('should return disabled field with initialValue true', () => { - const result = getDisabledField({ - initialValue: true, - disabled: false, - }); + it('should return SWITCH type with disabled prop', () => { + const result = getDisabledField({ initialValue: false, disabled: false }); - expect(result.props?.initialValue).toBe(true); - }); - - it('should return disabled field with disabled true', () => { - const result = getDisabledField({ - initialValue: false, - disabled: true, - }); - - expect(result.props?.isDisabled).toBe(true); + expect(result.type).toBe(FieldTypes.SWITCH); + expect(result.name).toBe('disabled'); + expect(result.required).toBe(false); + expect(result.props?.['data-testid']).toBe('disabled'); }); - it('should have horizontal form item layout', () => { - const result = getDisabledField({ - initialValue: false, - disabled: false, - }); - - expect(result.formItemLayout).toBe(FormItemLayout.HORIZONTAL); + it('should forward disabled prop', () => { + expect(getDisabledField({ initialValue: false, disabled: true }).props?.disabled).toBe(true); }); }); describe('getMutuallyExclusiveField', () => { - it('should return mutually exclusive field configuration with helper text hidden', () => { - const result = getMutuallyExclusiveField({ - disabled: false, - showHelperText: false, - }); - - expect(result).toEqual({ - name: 'mutuallyExclusive', - label: 'label.mutually-exclusive', - type: FieldTypes.UT_SWITCH, - required: false, - props: { - id: 'tags_mutuallyExclusive', - 'data-testid': 'mutually-exclusive-button', - isDisabled: false, - className: 'mutually-exclusive-switch', - }, - helperTextType: HelperTextType.ALERT, - showHelperText: false, - id: 'root/mutuallyExclusive', - }); - }); - - it('should return mutually exclusive field with helper text shown', () => { - const result = getMutuallyExclusiveField({ - disabled: false, - showHelperText: true, - }); - - expect(result.showHelperText).toBe(true); - }); + it('should return SWITCH type with ALERT helper text type', () => { + const result = getMutuallyExclusiveField({ disabled: false }); - it('should return mutually exclusive field with disabled true', () => { - const result = getMutuallyExclusiveField({ - disabled: true, - showHelperText: false, - }); - - expect(result.props?.isDisabled).toBe(true); + expect(result.type).toBe(FieldTypes.SWITCH); + expect(result.name).toBe('mutuallyExclusive'); + expect(result.required).toBe(false); + expect(result.helperTextType).toBe(HelperTextType.ALERT); + expect(result.props?.['data-testid']).toBe('mutually-exclusive-button'); }); - it('should have ALERT helper text type', () => { + it('should set helperText when provided', () => { const result = getMutuallyExclusiveField({ disabled: false, - showHelperText: false, + helperText: 'Alert message', }); - expect(result.helperTextType).toBe(HelperTextType.ALERT); + expect(result.helperText).toBe('Alert message'); }); - it('should be an optional field', () => { - const result = getMutuallyExclusiveField({ - disabled: false, - showHelperText: false, - }); - - expect(result.required).toBe(false); + it('should forward disabled prop', () => { + expect(getMutuallyExclusiveField({ disabled: true }).props?.disabled).toBe(true); }); }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/tagFormFields.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/tagFormFields.tsx index 240920639448..1ea57bf785bd 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/tagFormFields.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/tagFormFields.tsx @@ -10,159 +10,128 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DEFAULT_TAG_ICON } from '../../components/common/IconPicker'; import { FieldProp, FieldTypes, - FormItemLayout, HelperTextType, -} from '../../interface/FormUtils.interface'; -import { iconTooltipDataRender } from '../../utils/DomainUtils'; +} from '@openmetadata/ui-core-components'; +import { ReactNode } from 'react'; +import { DEFAULT_TAG_ICON } from '../../components/common/IconPicker'; +import { TagFormSelectItem } from './TagsPage.interface'; -export const getIconField = (selectedColor?: string): FieldProp => ({ - name: ['style', 'iconURL'], +export const getIconField = ( + selectedColor?: string, + iconOptions?: TagFormSelectItem[] +): FieldProp => ({ + name: 'style.iconURL', id: 'root/style/iconURL', label: 'label.icon', - muiLabel: 'label.icon', required: false, - type: FieldTypes.ICON_PICKER_MUI, - helperText: iconTooltipDataRender(), + type: FieldTypes.ICON_PICKER, placeholder: 'label.icon-url', - formItemLayout: FormItemLayout.HORIZONTAL, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, props: { 'data-testid': 'icon-picker-btn', allowUrl: true, backgroundColor: selectedColor, defaultIcon: DEFAULT_TAG_ICON, - customStyles: { - searchBoxWidth: 366, + options: iconOptions ?? [], + labels: { + customIconUrl: 'label.icon-url', + emptyState: 'message.no-entity-available', + enterIconUrl: 'label.enter-entity', + iconsTab: 'label.icon-plural', + urlTab: 'label.url', }, }, }); export const COLOR_FIELD: FieldProp = { - name: ['style', 'color'], + name: 'style.color', id: 'root/style/color', label: 'label.color', - muiLabel: 'label.color', required: false, - type: FieldTypes.COLOR_PICKER_MUI, - formItemLayout: FormItemLayout.HORIZONTAL, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, + type: FieldTypes.COLOR_PICKER, }; export const getNameField = (disabled: boolean): FieldProp => ({ name: 'name', id: 'root/name', - muiLabel: 'label.name', label: 'label.name', required: true, placeholder: 'label.name', - type: FieldTypes.TEXT_MUI, + type: FieldTypes.TEXT, props: { - inputProps: { - 'data-testid': 'name', - }, + 'data-testid': 'name', disabled, }, - formItemProps: { - validateTrigger: ['onChange', 'onBlur'], - }, }); export const getDisplayNameField = (disabled: boolean): FieldProp => ({ name: 'displayName', id: 'root/displayName', - muiLabel: 'label.display-name', label: 'label.display-name', required: false, placeholder: 'label.display-name', - type: FieldTypes.TEXT_MUI, + type: FieldTypes.TEXT, props: { - inputProps: { - 'data-testid': 'displayName', - }, + 'data-testid': 'displayName', disabled, }, }); export const getOwnerField = ({ - canAddMultipleUserOwners, - canAddMultipleTeamOwner, + multiple, + options, + onFocus, + onSearchChange, }: { - canAddMultipleUserOwners: boolean; - canAddMultipleTeamOwner: boolean; + multiple: boolean; + options: TagFormSelectItem[]; + onFocus: () => void; + onSearchChange: (searchText: string) => void; }): FieldProp => ({ name: 'owners', - id: 'root/owner', + id: 'root/owners', required: false, label: 'label.owner-plural', - muiLabel: 'label.owner-plural', - type: FieldTypes.USER_TEAM_SELECT_MUI, + type: FieldTypes.USER_TEAM_SELECT_INPUT, props: { - multipleUser: canAddMultipleUserOwners, - multipleTeam: canAddMultipleTeamOwner, - }, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', + filterOption: () => true, + multiple, + onFocus, + onSearchChange, + options, }, }); export const getDomainField = ({ canAddMultipleDomains, + options, + onFocus, + onSearchChange, }: { canAddMultipleDomains: boolean; + options: TagFormSelectItem[]; + onFocus: () => void; + onSearchChange: (searchText: string) => void; }): FieldProp => ({ name: 'domains', id: 'root/domains', required: false, label: 'label.domain-plural', - muiLabel: 'label.domain-plural', - type: FieldTypes.DOMAIN_SELECT_MUI, + type: FieldTypes.DOMAIN_SELECT, props: { 'data-testid': 'domain-select', - hasPermission: true, + filterOption: () => true, multiple: canAddMultipleDomains, - }, - formItemProps: { - valuePropName: 'value', - trigger: 'onChange', - }, -}); - -export const getDescriptionField = ({ - initialValue, - readonly, -}: { - initialValue: string; - readonly: boolean; -}): FieldProp => ({ - name: 'description', - required: true, - label: 'label.description', - id: 'root/description', - type: FieldTypes.DESCRIPTION, - props: { - 'data-testid': 'description', - initialValue, - readonly, - className: 'description-text-area', - }, - formItemProps: { - className: 'description-form-item', + onFocus, + onSearchChange, + options, }, }); export const getDisabledField = ({ - initialValue, + initialValue: _initialValue, disabled, }: { initialValue: boolean; @@ -172,33 +141,29 @@ export const getDisabledField = ({ required: false, label: 'label.disable-tag', id: 'root/disabled', - type: FieldTypes.UT_SWITCH, - formItemLayout: FormItemLayout.HORIZONTAL, + type: FieldTypes.SWITCH, props: { 'data-testid': 'disabled', - initialValue, - isDisabled: disabled, + disabled, }, }); export const getMutuallyExclusiveField = ({ disabled, - showHelperText, + helperText, }: { disabled: boolean; - showHelperText: boolean; + helperText?: ReactNode; }): FieldProp => ({ name: 'mutuallyExclusive', label: 'label.mutually-exclusive', - type: FieldTypes.UT_SWITCH, + type: FieldTypes.SWITCH, required: false, + helperTextType: HelperTextType.ALERT, + helperText, + id: 'root/mutuallyExclusive', props: { - id: 'tags_mutuallyExclusive', 'data-testid': 'mutually-exclusive-button', - isDisabled: disabled, - className: 'mutually-exclusive-switch', + disabled, }, - helperTextType: HelperTextType.ALERT, - showHelperText, - id: 'root/mutuallyExclusive', }); From a4f47a1bbdc01f600c53f2fee9c56160475ee51c Mon Sep 17 00:00:00 2001 From: Rohit0301 Date: Mon, 6 Jul 2026 19:43:32 +0530 Subject: [PATCH 2/4] fixed the UI --- .../ui/src/pages/TagsPage/TagsForm.tsx | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx index 0d057ff027be..0f744bb36805 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.tsx @@ -25,8 +25,8 @@ import { import { Users01 } from '@untitledui/icons'; import { debounce } from 'lodash'; import { Suspense, useCallback, useEffect, useMemo, useState } from 'react'; -import { useTranslation } from 'react-i18next'; import { useWatch } from 'react-hook-form'; +import { useTranslation } from 'react-i18next'; import { EntityAttachmentProvider } from '../../components/common/EntityDescription/EntityAttachmentProvider/EntityAttachmentProvider'; import { AVAILABLE_ICONS, @@ -53,11 +53,11 @@ import tagClassBase from '../../utils/TagClassBase'; import { COLOR_FIELD, getDisabledField, + getDisplayNameField, getDomainField, getIconField, getMutuallyExclusiveField, getNameField, - getDisplayNameField, getOwnerField, } from './tagFormFields'; import './TagsForm.less'; @@ -468,6 +468,7 @@ const TagsForm = ({ data-testid="tags-form" form={form} onSubmit={form.handleSubmit(handleSave)}> + {getField(nameField)} {getField(displayNameField)} @@ -498,8 +499,8 @@ const TagsForm = ({ className="tw:gap-[6px]" direction="col"> {getField(mutuallyExclusiveField)} + getField(mutuallyExclusiveField) )} - - - {getField(ownerField)} - {getField(domainField)} - - + {getField(ownerField)} + {getField(domainField)} {autoClassificationComponent && ( {autoClassificationComponent} )} + ); From 6b3479f8ef0aff536893f968e3b2690705e4bfd2 Mon Sep 17 00:00:00 2001 From: Rohit0301 Date: Mon, 6 Jul 2026 22:54:22 +0530 Subject: [PATCH 3/4] addressed gitar comment --- .../MUIUserTeamSelect/MUIUserTeamSelect.tsx | 435 ------------------ .../ui/src/pages/TagsPage/TagsForm.test.tsx | 134 ++++-- .../ui/src/pages/TagsPage/TagsForm.tsx | 91 ++-- .../main/resources/ui/src/utils/formUtils.tsx | 24 +- 4 files changed, 151 insertions(+), 533 deletions(-) delete mode 100644 openmetadata-ui/src/main/resources/ui/src/components/common/MUIUserTeamSelect/MUIUserTeamSelect.tsx diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/MUIUserTeamSelect/MUIUserTeamSelect.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/MUIUserTeamSelect/MUIUserTeamSelect.tsx deleted file mode 100644 index fa87938ec826..000000000000 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/MUIUserTeamSelect/MUIUserTeamSelect.tsx +++ /dev/null @@ -1,435 +0,0 @@ -/* - * Copyright 2024 Collate. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - Autocomplete, - AutocompleteRenderGetTagProps, - Box, - Chip, - TextField, - useTheme, -} from '@mui/material'; -import { XClose } from '@untitledui/icons'; -import { debounce } from 'lodash'; -import { - ComponentPropsWithoutRef, - FC, - HTMLAttributes, - SyntheticEvent, - useCallback, - useEffect, - useMemo, - useState, -} from 'react'; -import { useTranslation } from 'react-i18next'; -import { ReactComponent as IconTeams } from '../../../assets/svg/teams-grey.svg'; -import { PAGE_SIZE_MEDIUM } from '../../../constants/constants'; -import { EntityType } from '../../../enums/entity.enum'; -import { SearchIndex } from '../../../enums/search.enum'; -import { EntityReference } from '../../../generated/entity/type'; -import { searchData } from '../../../rest/miscAPI'; -import { - formatTeamsResponse, - formatUsersResponse, -} from '../../../utils/APIUtils'; -import { getEntityName } from '../../../utils/EntityNameUtils'; -import { getEntityReferenceFromEntity } from '../../../utils/EntityReferenceUtils'; -import { ProfilePicture } from '../atoms/ProfilePicture'; - -export interface MUIUserTeamSelectProps { - // Entity type restrictions - userOnly?: boolean; - teamOnly?: boolean; - - // Multiple selection control - multipleUser?: boolean; - multipleTeam?: boolean; - - // Common props - placeholder?: string; - value?: EntityReference[]; - onChange?: (selected: EntityReference[]) => void; - autoFocus?: boolean; - label?: React.ReactNode; - required?: boolean; -} - -interface OptionType { - label: string; - value: string; - entity: EntityReference; - isTeam: boolean; -} - -const MUIUserTeamSelect: FC = ({ - userOnly = false, - teamOnly = false, - multipleUser = false, - multipleTeam = false, - placeholder, - value = [], - onChange, - autoFocus, - label, - required, -}) => { - const { t } = useTranslation(); - const theme = useTheme(); - const [userOptions, setUserOptions] = useState([]); - const [teamOptions, setTeamOptions] = useState([]); - const [loading, setLoading] = useState(false); - const [inputValue, setInputValue] = useState(''); - const [open, setOpen] = useState(false); - - const selectedOptions = useMemo(() => { - return value.map((entity) => ({ - label: getEntityName(entity), - value: entity.id || '', - entity, - isTeam: entity.type === EntityType.TEAM, - })); - }, [value]); - - const fetchUsers = async (searchText: string) => { - if (teamOnly) { - setUserOptions([]); - - return; - } - - const res = await searchData( - searchText, - 1, - PAGE_SIZE_MEDIUM, - 'isBot:false', - '', - '', - SearchIndex.USER, - false, - false, - true - ); - - const users = formatUsersResponse(res.data.hits.hits); - - setUserOptions( - users.map((user) => ({ - label: getEntityName(user), - value: user.id, - entity: user as unknown as EntityReference, - isTeam: false, - })) - ); - }; - - const fetchTeams = async (searchText: string) => { - if (userOnly) { - setTeamOptions([]); - - return; - } - - const res = await searchData( - searchText, - 1, - PAGE_SIZE_MEDIUM, - '', - '', - '', - SearchIndex.TEAM, - false, - false, - true - ); - - const teams = formatTeamsResponse(res.data.hits.hits); - - setTeamOptions( - teams.map((team) => ({ - label: getEntityName(team), - value: team.id, - entity: team as unknown as EntityReference, - isTeam: true, - })) - ); - }; - - const handleSearch = useCallback( - debounce(async (searchText: string) => { - setLoading(true); - - try { - await Promise.all([fetchUsers(searchText), fetchTeams(searchText)]); - } catch (error) { - setUserOptions([]); - setTeamOptions([]); - } finally { - setLoading(false); - } - }, 300), - [userOnly, teamOnly] - ); - - useEffect(() => { - if (inputValue) { - handleSearch(inputValue); - } else { - setLoading(true); - setUserOptions([]); - setTeamOptions([]); - handleSearch(''); - } - }, [inputValue]); - - // Fetch initial options when dropdown opens - useEffect(() => { - if ( - open && - userOptions.length === 0 && - teamOptions.length === 0 && - !inputValue - ) { - handleSearch(''); - } - }, [open]); - - const handleChange = ( - _event: SyntheticEvent, - newValue: string | OptionType | (string | OptionType)[] | null - ) => { - if (!onChange) { - return; - } - - // Filter out string values from freeSolo - if (typeof newValue === 'string') { - return; - } - - if (Array.isArray(newValue)) { - // Multiple selection mode - handle team/user exclusivity - // Filter out any string values - const optionValues = newValue.filter( - (v): v is OptionType => typeof v !== 'string' - ); - let finalSelection = [...optionValues]; - - // Check if a new team was just added (comparing with previous selection) - const newTeams = optionValues.filter((opt) => opt.isTeam); - const oldTeams = selectedOptions.filter((opt) => opt.isTeam); - const teamWasAdded = newTeams.length > oldTeams.length; - - // Check if a new user was just added - const newUsers = optionValues.filter((opt) => !opt.isTeam); - const oldUsers = selectedOptions.filter((opt) => !opt.isTeam); - const userWasAdded = newUsers.length > oldUsers.length; - - if (teamWasAdded) { - // When a team is selected, remove all users and keep only the latest team - finalSelection = finalSelection.filter((opt) => opt.isTeam); - if (!multipleTeam && finalSelection.length > 1) { - // Keep only the most recent team - finalSelection = [newTeams[newTeams.length - 1]]; - } - } else if (userWasAdded) { - // When a user is selected, remove all teams - finalSelection = finalSelection.filter((opt) => !opt.isTeam); - if (!multipleUser && finalSelection.length > 1) { - // Keep only the most recent user - finalSelection = [newUsers[newUsers.length - 1]]; - } - } - - // Clean entities to valid EntityReference format - const entities = finalSelection.map((opt) => - getEntityReferenceFromEntity( - opt.entity, - opt.isTeam ? EntityType.TEAM : EntityType.USER - ) - ); - onChange(entities); - } else if (newValue) { - // Single selection mode - clean entity - const cleanEntity = getEntityReferenceFromEntity( - newValue.entity, - newValue.isTeam ? EntityType.TEAM : EntityType.USER - ); - onChange([cleanEntity]); - } else { - onChange([]); - } - }; - - const isMultiple = useMemo(() => { - // If both user and team can be selected - if (!userOnly && !teamOnly) { - return multipleUser || multipleTeam; - } - // If only users - if (userOnly) { - return multipleUser; - } - // If only teams - if (teamOnly) { - return multipleTeam; - } - - return false; - }, [userOnly, teamOnly, multipleUser, multipleTeam]); - - const isOptionEqualToValue = (option: OptionType, val: OptionType) => { - return option.value === val.value; - }; - - const filterOptions = (opts: OptionType[]) => { - // No filtering - all options are always available for selection - // The handleChange function will handle the replacement logic - return opts; - }; - - const renderOption = ( - props: HTMLAttributes, - option: OptionType - ) => { - const { entity, isTeam } = option; - - return ( - - {isTeam ? ( - - ) : ( - - )} - {getEntityName(entity)} - - ); - }; - - const renderTags = ( - value: OptionType[], - getTagProps: AutocompleteRenderGetTagProps - ) => { - return value.map((option, index) => { - const { entity, isTeam } = option; - const tagProps = getTagProps({ index }); - - return ( - - ) : ( - - ) - } - color="secondary" - deleteIcon={} - key={entity.id} - label={getEntityName(entity)} - size="small" - sx={{ - borderRadius: '8px', - backgroundColor: 'transparent', - borderColor: theme.palette.grey[300], - }} - variant="outlined" - /> - ); - }); - }; - - const getPlaceholderText = () => { - if (placeholder) { - return placeholder; - } - if (userOnly) { - return t('label.select-field', { field: t('label.user-plural') }); - } - if (teamOnly) { - return t('label.select-field', { field: t('label.team-plural') }); - } - - return t('label.select-users-or-team'); - }; - - const allOptions = useMemo( - () => [...userOptions, ...teamOptions], - [userOptions, teamOptions] - ); - - return ( - > & { key: string } - } - autoFocus={autoFocus} - filterOptions={filterOptions} - getOptionLabel={(option) => - typeof option === 'string' ? option : option.label - } - inputValue={inputValue} - isOptionEqualToValue={isOptionEqualToValue} - loading={loading} - multiple={isMultiple} - open={open && (allOptions.length > 0 || loading)} - options={allOptions} - renderInput={(params) => ( - - )} - renderOption={renderOption} - renderTags={renderTags} - slotProps={{ - popper: { 'data-react-aria-top-layer': true } as object, - }} - sx={{ width: '100%' }} - value={isMultiple ? selectedOptions : selectedOptions[0] || null} - onChange={handleChange} - onClose={() => setOpen(false)} - onInputChange={(_, newValue) => setInputValue(newValue)} - onOpen={() => setOpen(true)} - /> - ); -}; - -export default MUIUserTeamSelect; diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx index 8761de5b1a75..f726bafa317d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TagsPage/TagsForm.test.tsx @@ -18,6 +18,10 @@ import TagsForm from './TagsForm'; import { TagFormValues } from './TagsPage.interface'; jest.mock('@openmetadata/ui-core-components', () => { + const { FieldTypes, HelperTextType } = jest.requireActual( + '@openmetadata/ui-core-components' + ); + const GridItem = ({ children }: { children: React.ReactNode }) => (
{children}
); @@ -26,10 +30,35 @@ jest.mock('@openmetadata/ui-core-components', () => { ); GridComponent.Item = GridItem; + const Toggle = ({ + isSelected, + onChange, + isDisabled, + className, + ...rest + }: { + isSelected?: boolean; + onChange?: (val: boolean) => void; + isDisabled?: boolean; + className?: string; + } & Record) => ( + , - Toggle: ({ - id, - isSelected, - onChange, - isDisabled, - className, - }: { - id?: string; - isSelected?: boolean; - onChange?: (val: boolean) => void; - isDisabled?: boolean; - className?: string; - }) => ( -