diff --git a/apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx b/apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx index 77ee3e750e762..2a6964462deaf 100644 --- a/apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx +++ b/apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx @@ -60,7 +60,6 @@ const CreateDiscussion = ({ watch, setValue, } = useForm({ - mode: 'onBlur', defaultValues: { name: nameSuggestion || '', parentRoom: '', diff --git a/apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts b/apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts new file mode 100644 index 0000000000000..833046de91201 --- /dev/null +++ b/apps/meteor/client/hooks/useFormSubmitWithDirtyCheck.ts @@ -0,0 +1,39 @@ +import { useToastMessageDispatch } from '@rocket.chat/ui-contexts'; +import { useCallback } from 'react'; +import { useTranslation } from 'react-i18next'; + +type UseFormSubmitOptions = { + isDirty: boolean; + noChangesMessage?: string; +}; + +/** + * A reusable hook for form submission that implements accessible form validation patterns. + * + * This hook wraps your form submission handler and: + * - Allows submission attempts in both create and edit modes (keeping buttons enabled for a11y) + * - Provides user-friendly feedback when trying to save an unchanged edit form + */ + +export const useFormSubmitWithDirtyCheck = ( + onSubmit: (data: TData) => Promise | void, + { isDirty, noChangesMessage = 'No_changes_to_save' }: UseFormSubmitOptions, +) => { + const { t } = useTranslation(); + const dispatchToastMessage = useToastMessageDispatch(); + + return useCallback( + async (data: TData): Promise => { + if (!!data && !isDirty) { + dispatchToastMessage({ + type: 'info', + message: t(noChangesMessage), + }); + return; + } + + await onSubmit(data); + }, + [isDirty, onSubmit, dispatchToastMessage, t, noChangesMessage], + ); +}; diff --git a/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx b/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx index ad0f0627247ee..9b28bc9bb8e41 100644 --- a/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx +++ b/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateChannelModal.tsx @@ -111,7 +111,6 @@ const CreateChannelModal = ({ teamId = '', mainRoom, onClose, reload }: CreateCh setValue, watch, } = useForm({ - mode: 'onBlur', defaultValues: { members: [], name: '', diff --git a/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx b/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx index de91a82e46f62..d508db3fda098 100644 --- a/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx +++ b/apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx @@ -38,7 +38,7 @@ const CreateDirectMessage = ({ onClose }: CreateDirectMessageProps) => { control, handleSubmit, formState: { isSubmitting, isValidating, errors }, - } = useForm({ mode: 'onBlur', defaultValues: { users: [] } }); + } = useForm({ defaultValues: { users: [] } }); const goToRoom = useGoToRoom(); diff --git a/apps/meteor/client/views/account/profile/AccountProfilePage.tsx b/apps/meteor/client/views/account/profile/AccountProfilePage.tsx index 02fa3622f061f..13e93a8529476 100644 --- a/apps/meteor/client/views/account/profile/AccountProfilePage.tsx +++ b/apps/meteor/client/views/account/profile/AccountProfilePage.tsx @@ -39,7 +39,7 @@ const AccountProfilePage = (): ReactElement => { const methods = useForm({ defaultValues: getProfileInitialValues(user), - mode: 'onBlur', + reValidateMode: 'onBlur', }); const { diff --git a/apps/meteor/client/views/admin/users/AdminUserForm.tsx b/apps/meteor/client/views/admin/users/AdminUserForm.tsx index 1bf10554609a5..d5f13b08f6b4d 100644 --- a/apps/meteor/client/views/admin/users/AdminUserForm.tsx +++ b/apps/meteor/client/views/admin/users/AdminUserForm.tsx @@ -120,7 +120,6 @@ const AdminUserForm = ({ userData, onReload, context, refetchUserFormData, roleD isNewUserPage, isVerificationNeeded: !!isVerificationNeeded, }), - mode: 'onBlur', }); const showVoipExtension = useShowVoipExtension(); diff --git a/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx b/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx index 65a5079795339..e57e44fe01b4b 100644 --- a/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx +++ b/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx @@ -50,7 +50,6 @@ const MessageForm = (props: MessageFormProps) => { setValue, } = useForm({ mode: 'onChange', - reValidateMode: 'onChange', defaultValues: { templateParameters: defaultValues?.templateParameters ?? {}, templateId: defaultValues?.templateId ?? '', diff --git a/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/RecipientForm/RecipientForm.tsx b/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/RecipientForm/RecipientForm.tsx index bddcc3c765188..0ecfed32d42d1 100644 --- a/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/RecipientForm/RecipientForm.tsx +++ b/apps/meteor/client/views/omnichannel/components/outboundMessage/components/OutboundMessageWizard/forms/RecipientForm/RecipientForm.tsx @@ -46,7 +46,6 @@ const RecipientForm = (props: RecipientFormProps) => { const { trigger, control, handleSubmit, formState, clearErrors, setValue } = useForm({ mode: 'onChange', - reValidateMode: 'onChange', defaultValues: { contactId: defaultValues?.contactId ?? '', providerId: defaultValues?.providerId ?? '', diff --git a/apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx b/apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx index 2901b147215df..3a91e9c23670b 100644 --- a/apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx +++ b/apps/meteor/client/views/omnichannel/contactInfo/EditContactInfo.tsx @@ -23,6 +23,7 @@ import AdvancedContactModal from './AdvancedContactModal'; import { useCreateContact } from './hooks/useCreateContact'; import { useEditContact } from './hooks/useEditContact'; import { hasAtLeastOnePermission } from '../../../../app/authorization/client'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; import { useHasLicenseModule } from '../../../hooks/useHasLicenseModule'; import { omnichannelQueryKeys } from '../../../lib/queryKeys'; import { ContactManagerInput } from '../additionalForms'; @@ -89,12 +90,11 @@ const EditContactInfo = ({ contactData, onClose, onCancel }: ContactNewEditProps const initialValue = getInitialValues(contactData); const { - formState: { errors, isSubmitting }, + formState: { errors, isSubmitting, isDirty }, control, watch, handleSubmit, } = useForm({ - mode: 'onBlur', reValidateMode: 'onBlur', defaultValues: initialValue, }); @@ -165,26 +165,31 @@ const EditContactInfo = ({ contactData, onClose, onCancel }: ContactNewEditProps const validateName = (v: string): string | boolean => (!v.trim() ? t('Required_field', { field: t('Name') }) : true); - const handleSave = async (data: ContactFormData): Promise => { - const { name, phones, emails, customFields, contactManager } = data; - - const payload = { - name, - phones: phones.map(({ phoneNumber }) => phoneNumber), - emails: emails.map(({ address }) => address), - customFields, - contactManager, - }; - - if (contactData) { - await editContact.mutateAsync({ contactId: contactData?._id, ...payload }); + const handleSave = useFormSubmitWithDirtyCheck( + async (data: ContactFormData): Promise => { + const { name, phones, emails, customFields, contactManager } = data; + + const payload = { + name, + phones: phones.map(({ phoneNumber }) => phoneNumber), + emails: emails.map(({ address }) => address), + customFields, + contactManager, + }; + + if (contactData) { + await editContact.mutateAsync({ contactId: contactData?._id, ...payload }); + await queryClient.invalidateQueries({ queryKey: omnichannelQueryKeys.contacts() }); + return; + } + + await createContact.mutateAsync(payload); await queryClient.invalidateQueries({ queryKey: omnichannelQueryKeys.contacts() }); - return; - } - - await createContact.mutateAsync(payload); - await queryClient.invalidateQueries({ queryKey: omnichannelQueryKeys.contacts() }); - }; + }, + { + isDirty, + }, + ); const formId = useId(); const nameField = useId(); diff --git a/apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx b/apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx index 5e9824755d0dd..dfc7a269cc1ac 100644 --- a/apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx +++ b/apps/meteor/client/views/omnichannel/customFields/EditCustomFields.tsx @@ -13,7 +13,6 @@ import { ToggleSwitch, Box, } from '@rocket.chat/fuselage'; -import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; import { ContextualbarTitle, ContextualbarHeader, @@ -28,6 +27,7 @@ import { FormProvider, useForm, Controller } from 'react-hook-form'; import { CustomFieldsAdditionalForm } from '../additionalForms'; import { useRemoveCustomField } from './useRemoveCustomField'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; import { omnichannelQueryKeys } from '../../../lib/queryKeys'; export type EditCustomFieldsFormData = { @@ -66,7 +66,9 @@ const EditCustomFields = ({ customFieldData, onClose }: { customFieldData?: Seri const handleDelete = useRemoveCustomField(); - const methods = useForm({ mode: 'onBlur', values: getInitialValues(customFieldData) }); + const methods = useForm({ + values: getInitialValues(customFieldData), + }); const { control, handleSubmit, @@ -75,25 +77,28 @@ const EditCustomFields = ({ customFieldData, onClose }: { customFieldData?: Seri const saveCustomField = useEndpoint('POST', '/v1/livechat/custom-fields.save'); - const handleSave = useEffectEvent(async ({ visibility, ...data }: EditCustomFieldsFormData) => { - try { - await saveCustomField({ - customFieldId: customFieldData?._id as unknown as string, - customFieldData: { - visibility: visibility ? 'visible' : 'hidden', - ...data, - }, - }); + const handleSave = useFormSubmitWithDirtyCheck( + async ({ visibility, ...data }: EditCustomFieldsFormData) => { + try { + await saveCustomField({ + customFieldId: customFieldData?._id as unknown as string, + customFieldData: { + visibility: visibility ? 'visible' : 'hidden', + ...data, + }, + }); - dispatchToastMessage({ type: 'success', message: t('Saved') }); - queryClient.invalidateQueries({ - queryKey: omnichannelQueryKeys.livechat.customFields(), - }); - onClose(); - } catch (error) { - dispatchToastMessage({ type: 'error', message: error }); - } - }); + dispatchToastMessage({ type: 'success', message: t('Saved') }); + queryClient.invalidateQueries({ + queryKey: omnichannelQueryKeys.livechat.customFields(), + }); + onClose(); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }, + { isDirty }, + ); const scopeOptions: SelectOption[] = useMemo( () => [ @@ -221,7 +226,7 @@ const EditCustomFields = ({ customFieldData, onClose }: { customFieldData?: Seri - diff --git a/apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx b/apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx index f29d60eace55f..d90c1f7b8e549 100644 --- a/apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx +++ b/apps/meteor/client/views/omnichannel/departments/EditDepartment.tsx @@ -17,7 +17,7 @@ import { FieldHint, Option, } from '@rocket.chat/fuselage'; -import { useDebouncedValue, useEffectEvent } from '@rocket.chat/fuselage-hooks'; +import { useDebouncedValue } from '@rocket.chat/fuselage-hooks'; import { validateEmail } from '@rocket.chat/tools'; import { Page, PageHeader, PageScrollableContentWithShadow } from '@rocket.chat/ui-client'; import { useToastMessageDispatch, useEndpoint, useRouter, usePermission } from '@rocket.chat/ui-contexts'; @@ -32,6 +32,7 @@ import type { EditDepartmentFormData } from './definitions'; import { formatAgentListPayload } from './utils/formatAgentListPayload'; import { formatEditDepartmentPayload } from './utils/formatEditDepartmentPayload'; import { getFormInitialValues } from './utils/getFormInititalValues'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; import { useHasLicenseModule } from '../../../hooks/useHasLicenseModule'; import { useRoomsList } from '../../../hooks/useRoomsList'; import { EeTextInput, EeTextAreaInput, EeNumberInput, DepartmentBusinessHours } from '../additionalForms'; @@ -68,8 +69,8 @@ function EditDepartment({ data, id, title, allowedToForwardData }: EditDepartmen register, control, handleSubmit, - formState: { errors, isValid, isDirty, isSubmitting }, - } = useForm({ mode: 'onChange', defaultValues: initialValues }); + formState: { errors, isDirty, isSubmitting }, + } = useForm({ defaultValues: initialValues }); const [fallbackFilter, setFallbackFilter] = useState(''); const [isUnitRequired, setUnitRequired] = useState(false); @@ -82,42 +83,43 @@ function EditDepartment({ data, id, title, allowedToForwardData }: EditDepartmen const updateDepartmentInfo = useEndpoint('PUT', '/v1/livechat/department/:_id', { _id: id || '' }); const saveDepartmentAgentsInfoOnEdit = useEndpoint('POST', `/v1/livechat/department/:_id/agents`, { _id: id || '' }); - const handleSave = useEffectEvent(async (data: EditDepartmentFormData) => { - try { - const { agentList } = data; - const payload = formatEditDepartmentPayload(data); - const departmentUnit = data.unit ? { _id: data.unit } : undefined; + const handleSave = useFormSubmitWithDirtyCheck( + async (data: EditDepartmentFormData) => { + try { + const { agentList } = data; + const payload = formatEditDepartmentPayload(data); + const departmentUnit = data.unit ? { _id: data.unit } : undefined; - if (id) { - await updateDepartmentInfo({ - department: payload, - agents: [], - departmentUnit, - }); + if (id) { + await updateDepartmentInfo({ + department: payload, + agents: [], + departmentUnit, + }); - const { agentList: initialAgentList } = initialValues; - const agentListPayload = formatAgentListPayload(initialAgentList, agentList); + const { agentList: initialAgentList } = initialValues; + const agentListPayload = formatAgentListPayload(initialAgentList, agentList); - if (agentListPayload.upsert.length > 0 || agentListPayload.remove.length > 0) { - await saveDepartmentAgentsInfoOnEdit(agentListPayload); + if (agentListPayload.upsert.length > 0 || agentListPayload.remove.length > 0) { + await saveDepartmentAgentsInfoOnEdit(agentListPayload); + } + } else { + await createDepartment({ + department: payload, + agents: agentList.map(({ agentId, count, order }) => ({ agentId, count, order })), + departmentUnit, + }); } - } else { - await createDepartment({ - department: payload, - agents: agentList.map(({ agentId, count, order }) => ({ agentId, count, order })), - departmentUnit, - }); - } - - queryClient.invalidateQueries({ queryKey: ['/v1/livechat/department/:_id', id] }); - dispatchToastMessage({ type: 'success', message: t('Saved') }); - router.navigate('/omnichannel/departments'); - } catch (error) { - dispatchToastMessage({ type: 'error', message: error }); - } - }); - const isFormValid = isValid && isDirty; + queryClient.invalidateQueries({ queryKey: ['/v1/livechat/department/:_id', id] }); + dispatchToastMessage({ type: 'success', message: t('Saved') }); + router.navigate('/omnichannel/departments'); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }, + { isDirty }, + ); const formId = useId(); const enabledField = useId(); @@ -140,7 +142,7 @@ function EditDepartment({ data, id, title, allowedToForwardData }: EditDepartmen router.navigate('/omnichannel/departments')}> - diff --git a/apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx b/apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx index 3a407477fe448..c6c40e222fa05 100644 --- a/apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx +++ b/apps/meteor/client/views/omnichannel/priorities/PriorityEditForm.tsx @@ -1,65 +1,59 @@ import type { ILivechatPriority, Serialized } from '@rocket.chat/core-typings'; -import { Field, FieldError, Button, Box, ButtonGroup } from '@rocket.chat/fuselage'; -import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; +import { Field, FieldError, FieldLabel, FieldRow, TextInput, Button, ButtonGroup, ContextualbarFooter } from '@rocket.chat/fuselage'; +import { ContextualbarScrollableContent } from '@rocket.chat/ui-client'; import type { TranslationKey } from '@rocket.chat/ui-contexts'; -import { useToastMessageDispatch } from '@rocket.chat/ui-contexts'; import type { ReactElement } from 'react'; -import { useState } from 'react'; +import { useId } from 'react'; import { Controller, useForm } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; -import StringSettingInput from '../../admin/settings/Setting/inputs/StringSettingInput'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; export type PriorityFormData = { name: string; reset: boolean }; export type PriorityEditFormProps = { data: Serialized; - onCancel: () => void; onSave: (values: PriorityFormData) => Promise; }; type PrioritySaveException = { success: false; error: TranslationKey | undefined }; -const PriorityEditForm = ({ data, onSave, onCancel }: PriorityEditFormProps): ReactElement => { - const dispatchToastMessage = useToastMessageDispatch(); +const PriorityEditForm = ({ data, onSave }: PriorityEditFormProps): ReactElement => { const { t } = useTranslation(); - const [isSaving, setSaving] = useState(false); const { name, i18n, dirty } = data; const defaultName = t(i18n); const { control, - getValues, - setValue, - formState: { errors, isValid, isDirty }, + formState: { errors, isDirty, isSubmitting }, setError, + setValue, handleSubmit, + watch, } = useForm({ - mode: 'onChange', defaultValues: data ? { name: dirty ? name : defaultName } : {}, }); - const handleSave = useEffectEvent(async () => { - const { name } = getValues(); + const currentName = watch('name'); - if (!isValid) { - return dispatchToastMessage({ type: 'error', message: t('Required_field', { field: t('Name') }) }); - } + const formId = useId(); + const nameFieldId = useId(); - try { - setSaving(true); - await onSave({ name, reset: name === defaultName }); - } catch (e) { - const { error } = e as PrioritySaveException; + const handleSave = useFormSubmitWithDirtyCheck( + async ({ name }: { name: string }) => { + try { + await onSave({ name, reset: name === defaultName }); + } catch (e) { + const { error } = e as PrioritySaveException; - if (error) { - setError('name', { message: t(error) }); + if (error) { + setError('name', { message: t(error) }); + } } - } finally { - setSaving(false); - } - }); + }, + { isDirty }, + ); const onReset = (): void => { setValue('name', defaultName, { @@ -69,39 +63,55 @@ const PriorityEditForm = ({ data, onSave, onCancel }: PriorityEditFormProps): Re }; return ( - - - v?.trim() !== '' }} - render={({ field: { value, onChange } }): ReactElement => ( - + + + + + {t('Name')} + + + + value?.trim() !== '' || t('Required_field', { field: t('Name') }), + }} + render={({ field: { value, onChange } }): ReactElement => ( + onChange((e.target as HTMLInputElement).value)} + aria-describedby={`${nameFieldId}-error`} + aria-invalid={Boolean(errors.name?.message)} + error={errors.name?.message} + /> + )} /> + + {errors.name && ( + + {errors.name.message} + )} - /> - {errors.name?.message} - - - - - - + + + + + + + + + + ); }; diff --git a/apps/meteor/client/views/omnichannel/priorities/PriorityList.tsx b/apps/meteor/client/views/omnichannel/priorities/PriorityList.tsx index b40ca1cadea1f..c78055b00abb2 100644 --- a/apps/meteor/client/views/omnichannel/priorities/PriorityList.tsx +++ b/apps/meteor/client/views/omnichannel/priorities/PriorityList.tsx @@ -28,7 +28,7 @@ const PriorityList = ({ priorityId, onClose, onSave }: PriorityListProps): React - + ); diff --git a/apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx b/apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx index e8a5954499d73..ef5f0b3555931 100644 --- a/apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx +++ b/apps/meteor/client/views/omnichannel/slaPolicies/SlaEdit.tsx @@ -1,11 +1,23 @@ import type { IOmnichannelServiceLevelAgreements, Serialized } from '@rocket.chat/core-typings'; -import { Field, FieldLabel, FieldRow, FieldError, TextInput, Button, Margins, Box, NumberInput } from '@rocket.chat/fuselage'; -import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; +import { + Field, + FieldLabel, + FieldRow, + FieldError, + TextInput, + Button, + NumberInput, + ButtonGroup, + ContextualbarFooter, +} from '@rocket.chat/fuselage'; import { ContextualbarScrollableContent } from '@rocket.chat/ui-client'; import { useToastMessageDispatch, useRoute, useTranslation, useEndpoint } from '@rocket.chat/ui-contexts'; import type { ReactElement } from 'react'; +import { useId } from 'react'; import { useController, useForm } from 'react-hook-form'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; + type SlaEditProps = { isNew?: boolean; slaId?: string; @@ -13,6 +25,12 @@ type SlaEditProps = { data?: Serialized; }; +type SlaEditFormData = { + name: string; + description?: string; + dueTimeInMinutes: number; +}; + function SlaEdit({ data, isNew, slaId, reload, ...props }: SlaEditProps): ReactElement { const slasRoute = useRoute('omnichannel-sla-policies'); const saveSLA = useEndpoint('POST', '/v1/livechat/sla'); @@ -20,16 +38,17 @@ function SlaEdit({ data, isNew, slaId, reload, ...props }: SlaEditProps): ReactE const dispatchToastMessage = useToastMessageDispatch(); const t = useTranslation(); - const { name, description, dueTimeInMinutes } = data || {}; - const { control, - getValues, - formState: { errors, isValid, isDirty }, + formState: { errors, isDirty }, + handleSubmit, reset, - } = useForm({ - mode: 'onChange', - defaultValues: { name, description, dueTimeInMinutes }, + } = useForm({ + defaultValues: { + name: data?.name || '', + description: data?.description || '', + dueTimeInMinutes: data?.dueTimeInMinutes || 0, + }, }); const { field: nameField } = useController({ @@ -50,73 +69,96 @@ function SlaEdit({ data, isNew, slaId, reload, ...props }: SlaEditProps): ReactE const { field: descField } = useController({ control, name: 'description' }); - const handleSave = useEffectEvent(async () => { - const { name, description, dueTimeInMinutes } = getValues(); + const formId = useId(); + const nameFieldId = useId(); + const descFieldId = useId(); + const dueTimeFieldId = useId(); - if (!isValid || !name || dueTimeInMinutes === undefined) { - return dispatchToastMessage({ type: 'error', message: t('Required_field') }); - } + const handleSave = useFormSubmitWithDirtyCheck( + async ({ name, description, dueTimeInMinutes }: SlaEditFormData) => { + try { + const payload = { name, description, dueTimeInMinutes: Number(dueTimeInMinutes) }; + if (slaId) { + await updateSLA(payload); + } else { + await saveSLA(payload); + } - try { - const payload = { name, description, dueTimeInMinutes: Number(dueTimeInMinutes) }; - if (slaId) { - await updateSLA(payload); - } else { - await saveSLA(payload); + dispatchToastMessage({ type: 'success', message: t('Saved') }); + reload(); + slasRoute.push({}); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); } - - dispatchToastMessage({ type: 'success', message: t('Saved') }); - reload(); - slasRoute.push({}); - } catch (error) { - dispatchToastMessage({ type: 'error', message: error }); - } - }); + }, + { isDirty }, + ); return ( - - - {t('Name')}* - - - - {errors.name?.message} - - - {t('Description')} - - - - - - {t('Estimated_wait_time_in_minutes')}* - - - - {errors.dueTimeInMinutes?.message} - - - - - - {!isNew && ( - - )} - - - - - - + <> + + + + {t('Name')} + + + + + {errors.name && ( + + {errors.name.message} + + )} + + + {t('Description')} + + + + + + + {t('Estimated_wait_time_in_minutes')} + + + + + {errors.dueTimeInMinutes && ( + + {errors.dueTimeInMinutes.message} + + )} + + + + + {!isNew && ( + + )} + + + + ); } diff --git a/apps/meteor/client/views/omnichannel/tags/TagEdit.tsx b/apps/meteor/client/views/omnichannel/tags/TagEdit.tsx index a8202f637b17b..5e506fc51ac27 100644 --- a/apps/meteor/client/views/omnichannel/tags/TagEdit.tsx +++ b/apps/meteor/client/views/omnichannel/tags/TagEdit.tsx @@ -1,6 +1,5 @@ import type { ILivechatDepartment, ILivechatTag, Serialized } from '@rocket.chat/core-typings'; import { Field, FieldLabel, FieldRow, FieldError, TextInput, Button, ButtonGroup, FieldGroup, Box } from '@rocket.chat/fuselage'; -import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; import { ContextualbarScrollableContent, ContextualbarFooter, @@ -15,6 +14,7 @@ import { useForm, Controller } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; import { useRemoveTag } from './useRemoveTag'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; import AutoCompleteDepartmentMultiple from '../components/AutoCompleteDepartmentMultiple'; type TagEditPayload = { @@ -44,7 +44,6 @@ const TagEdit = ({ tagData, currentDepartments, onClose }: TagEditProps) => { formState: { isDirty, errors }, handleSubmit, } = useForm({ - mode: 'onBlur', values: { name: name || '', description: description || '', @@ -52,25 +51,27 @@ const TagEdit = ({ tagData, currentDepartments, onClose }: TagEditProps) => { }, }); - const handleSave = useEffectEvent(async ({ name, description, departments }: TagEditPayload) => { - const departmentsId = departments?.map((dep) => dep.value) || ['']; + const handleSave = useFormSubmitWithDirtyCheck( + async ({ name, description, departments }: TagEditPayload) => { + const departmentsId = departments?.map((dep) => dep.value) || ['']; - try { - await saveTag({ - _id, - tagData: { name, description }, - ...(departmentsId.length > 0 && { tagDepartments: departmentsId }), - }); - dispatchToastMessage({ type: 'success', message: t('Saved') }); - queryClient.invalidateQueries({ - queryKey: ['livechat-tags'], - }); - } catch (error) { - dispatchToastMessage({ type: 'error', message: error }); - } finally { - onClose(); - } - }); + try { + await saveTag({ + _id, + tagData: { name, description }, + ...(departmentsId.length > 0 && { tagDepartments: departmentsId }), + }); + dispatchToastMessage({ type: 'success', message: t('Saved') }); + queryClient.invalidateQueries({ + queryKey: ['livechat-tags'], + }); + onClose(); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }, + { isDirty }, + ); const formId = useId(); const nameField = useId(); @@ -130,7 +131,7 @@ const TagEdit = ({ tagData, currentDepartments, onClose }: TagEditProps) => { - diff --git a/apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx b/apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx index 4137d0d01ccf4..2026ee08e34b5 100644 --- a/apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx +++ b/apps/meteor/client/views/omnichannel/triggers/EditTrigger.tsx @@ -9,12 +9,13 @@ import { } from '@rocket.chat/ui-client'; import { useToastMessageDispatch, useEndpoint } from '@rocket.chat/ui-contexts'; import { useMutation, useQueryClient } from '@tanstack/react-query'; -import { useId, useMemo } from 'react'; +import { useId } from 'react'; import { Controller, useFieldArray, useForm } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; import { ConditionForm } from './ConditionForm'; import { ActionForm } from './actions/ActionForm'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; export type TriggersPayload = { name: string; @@ -90,12 +91,8 @@ const EditTrigger = ({ triggerData, onClose }: { triggerData?: Serialized({ mode: 'onBlur', reValidateMode: 'onBlur', values: initValues }); - - // Alternative way of checking isValid in order to not trigger validation on every render - // https://github.com/react-hook-form/documentation/issues/944 - const isValid = useMemo(() => Object.keys(errors).length === 0, [errors]); + formState: { isSubmitting, errors, isDirty }, + } = useForm({ values: initValues }); const { fields: conditionsFields } = useFieldArray({ control, @@ -124,13 +121,16 @@ const EditTrigger = ({ triggerData, onClose }: { triggerData?: Serialized { - return saveTriggerMutation.mutateAsync({ - ...data, - _id: triggerData?._id, - actions: data.actions.map(getDefaultAction), - }); - }; + const handleSave = useFormSubmitWithDirtyCheck( + async (data: TriggersPayload) => { + await saveTriggerMutation.mutateAsync({ + ...data, + _id: triggerData?._id, + actions: data.actions.map(getDefaultAction), + }); + }, + { isDirty }, + ); return ( <> @@ -211,7 +211,7 @@ const EditTrigger = ({ triggerData, onClose }: { triggerData?: Serialized - diff --git a/apps/meteor/client/views/omnichannel/units/UnitEdit.tsx b/apps/meteor/client/views/omnichannel/units/UnitEdit.tsx index a14aba067cb80..08303f660522e 100644 --- a/apps/meteor/client/views/omnichannel/units/UnitEdit.tsx +++ b/apps/meteor/client/views/omnichannel/units/UnitEdit.tsx @@ -7,7 +7,6 @@ import type { } from '@rocket.chat/core-typings'; import type { SelectOption } from '@rocket.chat/fuselage'; import { FieldError, Field, TextInput, Button, Select, ButtonGroup, FieldGroup, Box, FieldLabel, FieldRow } from '@rocket.chat/fuselage'; -import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; import { ContextualbarScrollableContent, ContextualbarFooter, @@ -20,6 +19,7 @@ import { useQueryClient } from '@tanstack/react-query'; import { useId, useMemo } from 'react'; import { useForm, Controller } from 'react-hook-form'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; import AutoCompleteDepartmentMultiple from '../components/AutoCompleteDepartmentMultiple'; import AutoCompleteMonitors from '../components/AutoCompleteMonitors'; @@ -82,7 +82,6 @@ const UnitEdit = ({ unitData, unitMonitors, unitDepartments, onUpdate, onDelete, handleSubmit, watch, } = useForm({ - mode: 'onBlur', values: { name: unitData?.name || '', visibility: unitData?.visibility || '', @@ -93,36 +92,39 @@ const UnitEdit = ({ unitData, unitMonitors, unitDepartments, onUpdate, onDelete, const { departments, monitors } = watch(); - const handleSave = useEffectEvent(async ({ name, visibility }: UnitEditFormData) => { - const departmentsData = departments.map((department) => ({ departmentId: department.value })); + const handleSave = useFormSubmitWithDirtyCheck( + async ({ name, visibility }: UnitEditFormData) => { + const departmentsData = departments.map((department) => ({ departmentId: department.value })); - const monitorsData = monitors.map((monitor) => ({ - monitorId: monitor.value, - username: monitor.label, - })); + const monitorsData = monitors.map((monitor) => ({ + monitorId: monitor.value, + username: monitor.label, + })); - const payload = { - unitData: { name, visibility }, - unitMonitors: monitorsData, - unitDepartments: departmentsData, - }; + const payload = { + unitData: { name, visibility }, + unitMonitors: monitorsData, + unitDepartments: departmentsData, + }; - try { - if (_id && onUpdate) { - await onUpdate(payload); - } else { - await saveUnit(payload); - } + try { + if (_id && onUpdate) { + await onUpdate(payload); + } else { + await saveUnit(payload); + } - dispatchToastMessage({ type: 'success', message: t('Saved') }); - queryClient.invalidateQueries({ - queryKey: ['livechat-units'], - }); - onClose(); - } catch (error) { - dispatchToastMessage({ type: 'error', message: error }); - } - }); + dispatchToastMessage({ type: 'success', message: t('Saved') }); + queryClient.invalidateQueries({ + queryKey: ['livechat-units'], + }); + onClose(); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }, + { isDirty }, + ); const formId = useId(); const nameField = useId(); @@ -263,7 +265,7 @@ const UnitEdit = ({ unitData, unitMonitors, unitDepartments, onUpdate, onDelete, - diff --git a/apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx b/apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx index ccdb19f009d1f..eaa81123d4314 100644 --- a/apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx +++ b/apps/meteor/client/views/room/contextualBar/ExportMessages/ExportMessages.tsx @@ -61,7 +61,7 @@ const ExportMessages = () => { const { control, - formState: { errors, isSubmitting, isDirty }, + formState: { errors, isSubmitting, isDirty, isSubmitted }, watch, register, setValue, @@ -69,7 +69,6 @@ const ExportMessages = () => { clearErrors, reset, } = useForm({ - mode: 'onBlur', defaultValues: { type: isE2ERoom ? 'download' : 'email', dateFrom: '', @@ -146,8 +145,8 @@ const ExportMessages = () => { }, [type, selectedMessageStore]); useEffect(() => { - setValue('messagesCount', messageCount, { shouldDirty: true }); - }, [messageCount, setValue]); + setValue('messagesCount', messageCount, { shouldDirty: true, shouldValidate: isSubmitted }); + }, [messageCount, setValue, isSubmitted]); const { mutateAsync: exportAsPDF } = useExportMessagesAsPDFMutation(); @@ -298,6 +297,17 @@ const ExportMessages = () => { { + const additionalEmails = watch('additionalEmails'); + if (toUsers?.length > 0 || additionalEmails !== '') { + return undefined; + } + return t('Mail_Message_Missing_to'); + }, + }, + }} render={({ field: { value, onChange, onBlur, name } }) => ( { }} onBlur={onBlur} name={name} + aria-label={t('To_users')} + aria-describedby={`${toUsersField}-error`} + aria-invalid={Boolean(errors?.toUsers?.message)} + error={errors?.toUsers?.message} /> )} /> + {errors?.toUsers && ( + + {errors.toUsers.message} + + )} {t('To_additional_emails')} @@ -333,7 +352,7 @@ const ExportMessages = () => { return t('Mail_Message_Invalid_emails', { postProcess: 'sprintf', sprintf: [additionalEmails] }); }, - validateToUsers: (additionalEmails) => { + validateRecipient: (additionalEmails) => { if (additionalEmails !== '' || toUsers?.length > 0) { return undefined; } @@ -342,10 +361,16 @@ const ExportMessages = () => { }, }, }} - render={({ field }) => ( + render={({ field: { value, onChange, onBlur, name } }) => ( { + onChange(e); + clearErrors('toUsers'); + }} + onBlur={onBlur} + name={name} placeholder={t('Email_Placeholder_any')} addon={} aria-describedby={`${additionalEmailsField}-error`} @@ -400,7 +425,7 @@ const ExportMessages = () => { - diff --git a/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx b/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx index 59c198f669c17..4f23522d092f8 100644 --- a/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx +++ b/apps/meteor/client/views/room/contextualBar/Info/EditRoomInfo/EditRoomInfo.tsx @@ -106,7 +106,7 @@ const EditRoomInfo = ({ room, onClickClose, onClickBack }: EditRoomInfoProps) => handleSubmit, getFieldState, formState: { isDirty, dirtyFields, errors, isSubmitting }, - } = useForm({ mode: 'onBlur', defaultValues }); + } = useForm({ defaultValues }); const sysMesOptions: SelectOption[] = useMemo( () => MessageTypesValues.map(({ key, i18nLabel }) => [key, t(i18nLabel as TranslationKey)]), diff --git a/apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx b/apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx index ae152952039a4..d583e612425b7 100644 --- a/apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx +++ b/apps/meteor/client/views/root/MainLayout/RegisterUsername.tsx @@ -53,9 +53,7 @@ const RegisterUsername = () => { setError, control, formState: { errors }, - } = useForm({ - mode: 'onBlur', - }); + } = useForm(); useEffect(() => { if (data?.result && getValues('username') === '') { diff --git a/apps/meteor/tests/e2e/export-messages.spec.ts b/apps/meteor/tests/e2e/export-messages.spec.ts index 7338d15cb67b1..344dad4e723b5 100644 --- a/apps/meteor/tests/e2e/export-messages.spec.ts +++ b/apps/meteor/tests/e2e/export-messages.spec.ts @@ -73,7 +73,9 @@ test.describe('export-messages', () => { await expect(exportMessagesTab.getOutputFormatOptionByName('pdf')).toBeVisible(); }); - test('should display an error when trying to send email without filling to users or to additional emails', async ({ page }) => { + test('when trying to send email without filling to users or to additional emails, should mark both fields as invalid', async ({ + page, + }) => { const exportMessagesTab = new ExportMessagesTab(page); const testMessage = uniqueMessage(); @@ -87,11 +89,16 @@ test.describe('export-messages', () => { await poHomeChannel.content.getMessageByText(testMessage).click(); await exportMessagesTab.send(); - await expect( - page.locator('[role="alert"]', { - hasText: 'You must select one or more users or provide one or more email addresses, separated by commas', - }), - ).toBeVisible(); + const usersField = exportMessagesTab.inputUsers; + const additionalEmailsField = exportMessagesTab.inputAdditionalEmails; + + await expect(usersField).toHaveAttribute('aria-invalid', 'true'); + await expect(additionalEmailsField).toHaveAttribute('aria-invalid', 'true'); + + const errorMessages = exportMessagesTab.errorMessage( + 'You must select one or more users or provide one or more email addresses, separated by commas', + ); + await expect(errorMessages).toHaveCount(2); }); test('should display an error when trying to send email without selecting any message', async ({ page }) => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts index 14c54560a4af0..7ebda25b44abb 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-contacts.spec.ts @@ -132,7 +132,7 @@ test.describe('OC - Contact Center - Contacts', () => { await test.step('validate email format', async () => { await poContacts.editContact.btnAddEmail.click(); await poContacts.editContact.inputEmail.fill('invalidemail'); - await page.keyboard.press('Tab'); + await page.keyboard.press('Enter'); await expect(poContacts.editContact.getErrorMessage(ERROR.invalidEmail)).toBeVisible(); }); @@ -207,7 +207,7 @@ test.describe('OC - Contact Center - Contacts', () => { await test.step('validate email format', async () => { await poContacts.editContact.inputEmail.fill('invalidemail'); - await page.keyboard.press('Tab'); + await page.keyboard.press('Enter'); await expect(poContacts.editContact.getErrorMessage(ERROR.invalidEmail)).toBeVisible(); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.ts index 190f485067ba2..66b498292a6cf 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-departaments.spec.ts @@ -49,20 +49,9 @@ test.describe('OC - Manage Departments', () => { await test.step('expect name and email to be required', async () => { await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredName)).not.toBeVisible(); - await poOmnichannelDepartments.inputName.fill('any_text'); - await poOmnichannelDepartments.inputName.fill(''); + await poOmnichannelDepartments.btnSave.click(); await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredName)).toBeVisible(); - await poOmnichannelDepartments.inputName.fill('any_text'); - await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredName)).not.toBeVisible(); - - await poOmnichannelDepartments.inputEmail.fill('any_text'); - await expect(poOmnichannelDepartments.errorMessage(ERROR.invalidEmail)).toBeVisible(); - - await poOmnichannelDepartments.inputEmail.fill(''); await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredEmail)).toBeVisible(); - - await poOmnichannelDepartments.inputEmail.fill(faker.internet.email()); - await expect(poOmnichannelDepartments.errorMessage(ERROR.requiredEmail)).not.toBeVisible(); }); await test.step('expect to fill required fields', async () => { @@ -203,10 +192,6 @@ test.describe('OC - Manage Departments', () => { await poOmnichannelDepartments.getDepartmentMenuByName(department.name).click(); await poOmnichannelDepartments.menuEditOption.click(); - await test.step('should form save button be disabled', async () => { - await expect(poOmnichannelDepartments.btnSave).toBeDisabled(); - }); - await test.step('should be able to add a tag properly', async () => { await poOmnichannelDepartments.inputConversationClosingTags.fill(tagName); await poOmnichannelDepartments.btnAddTags.click(); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts index 45e8f911d59fa..dd2de63edb1bb 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-monitor-department.spec.ts @@ -116,11 +116,12 @@ test.describe.serial('OC - Monitor Role', () => { await test.step('expect unit field to be required', async () => { await poOmnichannelDepartments.inputUnit.click(); await poOmnichannelDepartments.findOption('None').click(); - await expect(poOmnichannelDepartments.btnSave).toBeDisabled(); + await poOmnichannelDepartments.btnSave.click(); await expect(poOmnichannelDepartments.errorMessage('Unit required')).toBeVisible(); + await poOmnichannelDepartments.inputUnit.click(); await poOmnichannelDepartments.findOption(unitB.name).click(); - await expect(poOmnichannelDepartments.btnSave).toBeEnabled(); + await expect(poOmnichannelDepartments.errorMessage('Unit required')).not.toBeVisible(); }); await test.step('expect to save department', async () => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.ts index 625af75f5afca..b399a0838f83f 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-priorities.spec.ts @@ -54,30 +54,30 @@ test.describe.serial('Omnichannel Priorities', () => { await test.step('default state', async () => { await Promise.all([ - expect(poOmnichannelPriorities.editPriority.btnSave).toBeDisabled(), - expect(poOmnichannelPriorities.editPriority.btnReset).not.toBeVisible(), + expect(poOmnichannelPriorities.editPriority.btnSave).not.toBeDisabled(), expect(poOmnichannelPriorities.editPriority.inputName).toHaveValue('Highest'), ]); }); await test.step('field name is required', async () => { await poOmnichannelPriorities.editPriority.inputName.fill('any_text'); - await expect(poOmnichannelPriorities.editPriority.btnSave).toBeEnabled(); - await poOmnichannelPriorities.editPriority.inputName.fill(''); + await poOmnichannelPriorities.editPriority.inputName.clear(); + await poOmnichannelPriorities.editPriority.btnSave.click(); + await expect(poOmnichannelPriorities.editPriority.errorMessage(ERROR.fieldNameRequired)).toBeVisible(); + }); + + await test.step('should trim field name and show error if only spaces', async () => { + await poOmnichannelPriorities.editPriority.inputName.fill(' '); + await poOmnichannelPriorities.editPriority.btnSave.click(); await expect(poOmnichannelPriorities.editPriority.errorMessage(ERROR.fieldNameRequired)).toBeVisible(); - await expect(poOmnichannelPriorities.editPriority.btnSave).toBeDisabled(); }); await test.step('edit and save priority', async () => { await poOmnichannelPriorities.editPriority.inputName.fill(PRIORITY_NAME); - await Promise.all([ - expect(poOmnichannelPriorities.editPriority.btnReset).toBeVisible(), - expect(poOmnichannelPriorities.editPriority.btnSave).toBeEnabled(), - ]); + await expect(poOmnichannelPriorities.editPriority.errorMessage(ERROR.fieldNameRequired)).not.toBeVisible(); await poOmnichannelPriorities.editPriority.save(); await Promise.all([ - expect(poOmnichannelPriorities.editPriority.inputName).not.toBeVisible(), expect(poOmnichannelPriorities.findPriority(PRIORITY_NAME)).toBeVisible(), expect(poOmnichannelPriorities.findPriority('Highest')).not.toBeVisible(), ]); @@ -91,13 +91,13 @@ test.describe.serial('Omnichannel Priorities', () => { await poOmnichannelPriorities.editPriority.btnReset.click(); await Promise.all([ expect(poOmnichannelPriorities.editPriority.inputName).toHaveValue('Highest'), - expect(poOmnichannelPriorities.editPriority.btnReset).not.toBeVisible(), + expect(poOmnichannelPriorities.editPriority.btnReset).toBeDisabled(), ]); await expect(poOmnichannelPriorities.editPriority.btnSave).toBeEnabled(); await poOmnichannelPriorities.editPriority.save(); await expect(poOmnichannelPriorities.findPriority('Highest')).toBeVisible(); - await expect(poOmnichannelPriorities.btnReset).not.toBeEnabled(); + await expect(poOmnichannelPriorities.btnReset).toBeDisabled(); }); await test.step('reset all', async () => { @@ -120,7 +120,7 @@ test.describe.serial('Omnichannel Priorities', () => { await poOmnichannelPriorities.resetPriorities(); await Promise.all([ - expect(poOmnichannelPriorities.btnReset).not.toBeEnabled(), + expect(poOmnichannelPriorities.btnReset).toBeDisabled(), expect(poOmnichannelPriorities.findPriority(PRIORITY_NAME)).not.toBeVisible(), expect(poOmnichannelPriorities.findPriority('Highest')).toBeVisible(), ]); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts index e467798cbac00..adfd715534886 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-sla-policies.spec.ts @@ -50,33 +50,29 @@ test.describe('Omnichannel SLA Policies', () => { await test.step('Add new SLA', async () => { await poOmnichannelSlaPolicies.createNew(); - await test.step('field name is required', async () => { - await poOmnichannelSlaPolicies.manageSlaPolicy.inputName.fill('any_text'); - await poOmnichannelSlaPolicies.manageSlaPolicy.inputName.fill(''); + await test.step('should not submit form with empty required fields: name and estimated wait time', async () => { + await poOmnichannelSlaPolicies.manageSlaPolicy.btnSave.click(); await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.nameRequired)).toBeVisible(); + await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.estimatedWaitTimeRequired)).toBeVisible(); }); await test.step('input a valid name', async () => { await poOmnichannelSlaPolicies.manageSlaPolicy.inputName.fill(INITIAL_SLA.name); await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.nameRequired)).not.toBeVisible(); - await expect(poOmnichannelSlaPolicies.manageSlaPolicy.btnSave).toBeDisabled(); }); await test.step('input a valid description', async () => { await poOmnichannelSlaPolicies.manageSlaPolicy.inputDescription.fill(INITIAL_SLA.description); - await expect(poOmnichannelSlaPolicies.manageSlaPolicy.btnSave).toBeDisabled(); }); await test.step('only allow numbers on estimated wait time field', async () => { - await poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime.type('a'); - await expect(poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime).toHaveValue(''); - await expect(poOmnichannelSlaPolicies.manageSlaPolicy.btnSave).toBeDisabled(); + await poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime.pressSequentially('a'); + await expect(poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime).toHaveValue('0'); }); await test.step('not allow 0 on estimated wait time field', async () => { await poOmnichannelSlaPolicies.manageSlaPolicy.inputEstimatedWaitTime.fill('0'); await expect(poOmnichannelSlaPolicies.manageSlaPolicy.errorMessage(ERROR.estimatedWaitTimeRequired)).toBeVisible(); - await expect(poOmnichannelSlaPolicies.manageSlaPolicy.btnSave).toBeDisabled(); }); await test.step('input a valid estimated wait time', async () => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts index 9275166c4eaa6..dd70cd34b4f0d 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-tags.spec.ts @@ -54,7 +54,7 @@ test.describe('OC - Manage Tags', () => { await test.step('expect correct form default state', async () => { await poOmnichannelTags.createNew(); await expect(poOmnichannelTags.editTag.root).toBeVisible(); - await expect(poOmnichannelTags.editTag.btnSave).toBeDisabled(); + await expect(poOmnichannelTags.editTag.btnSave).toBeEnabled(); await expect(poOmnichannelTags.editTag.btnCancel).toBeEnabled(); await poOmnichannelTags.editTag.btnCancel.click(); await expect(poOmnichannelTags.editTag.root).not.toBeVisible(); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts index f94c63addec63..5a22e504f6945 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-units.spec.ts @@ -66,7 +66,7 @@ test.describe('OC - Manage Units', () => { await test.step('expect correct form default state', async () => { await poOmnichannelUnits.createNew(); await expect(poOmnichannelUnits.manageUnit.root).toBeVisible(); - await expect(poOmnichannelUnits.manageUnit.btnSave).toBeDisabled(); + await expect(poOmnichannelUnits.manageUnit.btnSave).toBeEnabled(); await expect(poOmnichannelUnits.manageUnit.btnCancel).toBeEnabled(); await poOmnichannelUnits.manageUnit.btnCancel.click(); await expect(poOmnichannelUnits.manageUnit.root).not.toBeVisible(); diff --git a/apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts b/apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts index fc47a7ade63b6..edceaebf6878a 100644 --- a/apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts +++ b/apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts @@ -50,13 +50,21 @@ export class ExportMessagesTab extends FlexTab { } async setAdditionalEmail(email: string) { - await this.toAdditionalEmailsInput.fill(email); + await this.inputAdditionalEmails.fill(email); } getMessageCheckbox(messageText: string): Locator { return this.root.page().getByRole('listitem').filter({ hasText: messageText }).getByRole('checkbox'); } + get inputUsers() { + return this.root.getByLabel('To users'); + } + + get inputAdditionalEmails() { + return this.root.getByRole('textbox', { name: 'To additional emails' }); + } + get method() { return this.root.getByTestId('export-messages-method'); } @@ -65,10 +73,6 @@ export class ExportMessagesTab extends FlexTab { return this.root.page().getByTestId('export-messages-output-format'); } - get toAdditionalEmailsInput() { - return this.root.getByRole('textbox', { name: 'To additional emails' }); - } - get downloadButton() { return this.root.getByRole('button', { name: 'Download', exact: true }); } diff --git a/apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.ts b/apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.ts index 62a06fe2d75cf..277e5cbb97b4b 100644 --- a/apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.ts +++ b/apps/meteor/tests/e2e/page-objects/omnichannel/omnichannel-sla-policies.ts @@ -14,7 +14,7 @@ class OmnichannelManageSlaPolicyFlexTab extends FlexTab { } get inputEstimatedWaitTime(): Locator { - return this.root.locator('[name="dueTimeInMinutes"]'); + return this.root.getByRole('spinbutton', { name: 'Estimated wait time (time in minutes)', exact: true }); } } diff --git a/docs/form-validation.md b/docs/form-validation.md new file mode 100644 index 0000000000000..d280beb59539a --- /dev/null +++ b/docs/form-validation.md @@ -0,0 +1,235 @@ +# Form Validation Guidelines + +This document outlines the standardized form validation patterns and guidelines established in PR [#39590](https://github.com/RocketChat/Rocket.Chat/pull/39590) to ensure consistent user experience across Rocket.Chat forms. + +## Overview + +The form validation standardization aims to: +- **Improve accessibility** by keeping submit buttons enabled and letting validation run on submit +- **Provide consistent UX** with validation triggered on form submission and re-validation on field changes +- **Prevent unnecessary API calls** by using dirty-checks and appropriate revalidation modes +- **Enhance user feedback** with clear error messages and proper ARIA attributes + +## Core Principles + +### 1. Submit-First validation (`mode: 'onSubmit'`) + +Forms should use `mode: 'onSubmit'` in react-hook-form to trigger initial validation only when the user attempts to submit the form. + +**Why:** This approach improves accessibility by: +- Keeping submit buttons enabled (allowing screen readers and keyboard users to discover validation requirements) +- Avoiding premature error messages that can confuse users +- Letting users complete the form at their own pace before seeing validation feedback + +**Example:** +```tsx +const { + control, + formState: { errors, isDirty, isSubmitting }, + handleSubmit, +} = useForm({ + mode: 'onSubmit', // This can be omitted, `onSubmit` it's the default mode value + defaultValues: initialData, +}); +``` + +### 2. Smart revalidation strategy + +After the first submit attempt, forms should revalidate fields intelligently: + +#### Default: `reValidateMode: 'onChange'` +For most forms, use the default onChange revalidation to provide immediate feedback as users correct errors. + +#### Exception: `reValidateMode: 'onBlur'` for Async Validation +For forms with **async validation** (e.g., username availability, email uniqueness checks), explicitly set `reValidateMode: 'onBlur'` to avoid excessive API calls. + +**Example with async validation:** +```tsx +const { + control, + formState: { errors, isDirty }, + handleSubmit, +} = useForm({ + reValidateMode: 'onBlur', // Avoid API calls on every keystroke + defaultValues: initialData, +}); +``` + +### 3. Dirty-check with `useFormSubmitWithDirtyCheck` + +Use the `useFormSubmitWithDirtyCheck` hook to provide user-friendly feedback when attempting to save unchanged forms. + +Usually applicable on edit forms, where fields are already populated. + +**Purpose:** +- Prevents unnecessary save operations on unchanged data +- Shows informative toast message: "No changes to save" +- Maintains accessibility by keeping buttons enabled + +**Signature:** + +Receives a callback as the first parameter (your submit handler), and an object as the second parameter containing `isDirty` and an optional `noChangesMessage` translation key, to be dispatched in the info toast. + +**Usage:** + +```tsx +import { useFormSubmitWithDirtyCheck } from '/hooks/useFormSubmitWithDirtyCheck'; + +const handleSave = useFormSubmitWithDirtyCheck( + async (data: FormData) => { + try { + await saveData(data); + dispatchToastMessage({ type: 'success', message: t('Saved') }); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }, + { isDirty } +); + +// In JSX: +
+``` + +**When to use dirty-check:** + +This hook is recommended when the same form component is used for both creation (new) and editing existing data. The hook intelligently handles both scenarios: +- ✅ **Create mode** (no existing data): Allows submission without dirty check +- ✅ **Edit mode** (existing data): Shows "No changes to save" toast when form is unchanged +- ✅ **Unified component**: Simplifies logic by handling both create and edit in one place + + +## Form Implementation Patterns + +### Basic Form Structure + +```tsx +import { useForm, Controller } from 'react-hook-form'; +import { useFormSubmitWithDirtyCheck } from '../../../hooks/useFormSubmitWithDirtyCheck'; + +type FormData = { + name: string; + email: string; +}; + +const MyForm = ({ data, onSave }: FormProps) => { + const { t } = useTranslation(); + + + const { + control, + formState: { errors, isDirty, isSubmitting }, + handleSubmit, + } = useForm({ + defaultValues: data || {}, + }); + + const handleFormSubmit = useFormSubmitWithDirtyCheck( + async (formData: FormData) => { + await onSave(formData); + }, + { isDirty } + ); + + return ( + + {/* Form fields */} + + ); +}; +``` + +## Button State Management + +### Submit Button States + +```tsx + +``` + +**Key Points:** +- Use `loading={isSubmitting}` to show loading state during submission +- Never disable the save button (keep enabled for a11y) +- Always connect button to form via `form={formId}` attribute + + + +## Basic checklist + +When updating an existing form to follow these guidelines: + +- [ ] Use `mode` to `'onSubmit'` in `useForm` +- [ ] Add `reValidateMode: 'onBlur'` if form has async validation +- [ ] Wrap submit handler with `useFormSubmitWithDirtyCheck` (for create and edit forms) +- [ ] Add ARIA attributes: `aria-describedby`, `aria-invalid`, `role='alert'` when applicable +- [ ] Button states: `loading={isSubmitting}`, but never `disabled` +- [ ] Verify accessibility with screen reader testing + +## Basic DOs and DON'Ts + +### ❌ Don't: Disable buttons based on form validity + +```tsx +// Bad - prevents discovery of validation requirements + +``` + +### ✅ Do: Keep buttons enabled, let validation run on submit + +```tsx +// Good - accessible and provides feedback + +``` + +### ❌ Don't: Use `mode: 'onChange'` for initial validation + +```tsx +// Bad - shows errors immediately, poor UX +useForm({ mode: 'onChange' }) +``` + +### ✅ Do: Use `mode: 'onSubmit'` for initial validation + +```tsx +// Good - validates on submit, revalidates on change +useForm({ mode: 'onSubmit' }) +``` + +### ❌ Don't: Use `reValidateMode: 'onChange'` with async validation + +```tsx +// Bad - causes API call on every keystroke +useForm({ + mode: 'onSubmit', + // Uses default 'onChange' revalidation - too many API calls! +}) +``` + +### ✅ Do: Use `reValidateMode: 'onBlur'` with async validation + +```tsx +// Good - reduces API calls while maintaining feedback +useForm({ + mode: 'onSubmit', + reValidateMode: 'onBlur', +}) +``` + +## Additional Resources + +- [React Hook Form Documentation](https://react-hook-form.com/) +- [WCAG 2.1 Form Guidelines](https://www.w3.org/WAI/WCAG21/quickref/?showtechniques=332#error-identification) +- [PR #39590 - Form Validation Standardization](https://github.com/RocketChat/Rocket.Chat/pull/39590) diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index b8e5891f1c420..865c7d7a181be 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -7155,5 +7155,6 @@ "Date_range_presets": "Date range presets", "message_body": "message body", "message_attachment": "message attachment", - "system_message_body": "system message body" + "system_message_body": "system message body", + "No_changes_to_save": "No changes to save" } \ No newline at end of file diff --git a/packages/web-ui-registration/src/LoginForm.tsx b/packages/web-ui-registration/src/LoginForm.tsx index 39aeac58e7c8e..e9217dd5e15dd 100644 --- a/packages/web-ui-registration/src/LoginForm.tsx +++ b/packages/web-ui-registration/src/LoginForm.tsx @@ -64,10 +64,7 @@ export const LoginForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRoute clearErrors, getValues, formState: { errors }, - } = useForm<{ usernameOrEmail: string; password: string }>({ - mode: 'onSubmit', - reValidateMode: 'onChange', - }); + } = useForm<{ usernameOrEmail: string; password: string }>(); const watchUsernameOrEmail = watch('usernameOrEmail'); const watchPassword = watch('password'); diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index c86b53a34e306..ca3e87c101254 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -69,7 +69,7 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo clearErrors, control, formState: { errors }, - } = useForm({ mode: 'onBlur' }); + } = useForm(); const { password } = watch(); const passwordIsValid = useValidatePassword(password); diff --git a/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx b/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx index 96ebcfdbfecba..680e65584c60d 100644 --- a/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx +++ b/packages/web-ui-registration/src/ResetPassword/ResetPasswordPage.tsx @@ -56,9 +56,7 @@ const ResetPasswordPage = (): ReactElement => { } = useForm<{ password: string; passwordConfirmation: string; - }>({ - mode: 'onBlur', - }); + }>(); const password = watch('password'); const passwordIsValid = useValidatePassword(password); diff --git a/packages/web-ui-registration/src/ResetPasswordForm.tsx b/packages/web-ui-registration/src/ResetPasswordForm.tsx index 86a43330efda0..b56b8867bc2af 100644 --- a/packages/web-ui-registration/src/ResetPasswordForm.tsx +++ b/packages/web-ui-registration/src/ResetPasswordForm.tsx @@ -23,7 +23,7 @@ export const ResetPasswordForm = ({ setLoginRoute }: { setLoginRoute: DispatchLo formState: { errors, isSubmitting }, } = useForm<{ email: string; - }>({ mode: 'onBlur' }); + }>(); useEffect(() => { if (forgotPasswordFormRef.current) {