From 92b57b4bdfdcde4c7257d3f1ef8e8f5c4eae8e37 Mon Sep 17 00:00:00 2001 From: Simon Backx Date: Fri, 6 Oct 2023 14:23:18 +0200 Subject: [PATCH] Improved error handling in recommendation modals (#18524) fixes https://github.com/TryGhost/Product/issues/3990 - Show errors on blur - Show toast on submit (final modal only) - Do an initial validation on mount --- .../admin-x-ds/global/form/URLTextField.tsx | 2 +- .../AddRecommendationModal.tsx | 39 +++++++---- .../AddRecommendationModalConfirm.tsx | 25 +++---- .../EditRecommendationModal.tsx | 25 ++++--- .../RecommendationReasonForm.tsx | 57 +++++++++++++++- apps/admin-x-settings/src/hooks/useForm.ts | 66 +++++++++++-------- 6 files changed, 143 insertions(+), 71 deletions(-) diff --git a/apps/admin-x-settings/src/admin-x-ds/global/form/URLTextField.tsx b/apps/admin-x-settings/src/admin-x-ds/global/form/URLTextField.tsx index 94cf983b94..51be570bad 100644 --- a/apps/admin-x-settings/src/admin-x-ds/global/form/URLTextField.tsx +++ b/apps/admin-x-settings/src/admin-x-ds/global/form/URLTextField.tsx @@ -26,7 +26,7 @@ export const formatUrl = (value: string, baseUrl?: string) => { if (!baseUrl) { // Absolute URL with no base URL - if (!url.startsWith('http://') && !url.startsWith('https://')) { + if (!url.startsWith('http')) { url = `https://${url}`; } } diff --git a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx index afff039118..faab2694e5 100644 --- a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx +++ b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx @@ -4,7 +4,7 @@ import Modal from '../../../../admin-x-ds/global/modal/Modal'; import NiceModal, {useModal} from '@ebay/nice-modal-react'; import React, {useEffect, useState} from 'react'; import TextField from '../../../../admin-x-ds/global/form/TextField'; -import useForm from '../../../../hooks/useForm'; +import useForm, {ErrorMessages} from '../../../../hooks/useForm'; import useRouting from '../../../../hooks/useRouting'; import {AlreadyExistsError} from '../../../../utils/errors'; import {EditOrAddRecommendation, RecommendationResponseType, useGetRecommendationByUrl} from '../../../../api/recommendations'; @@ -25,6 +25,22 @@ const doFormatUrl = (url: string) => { return formatUrl(url).save; }; +const validateUrl = function (errors: ErrorMessages, url: string) { + try { + const u = new URL(url); + + // Check domain includes a dot + if (!u.hostname.includes('.')) { + errors.url = 'Please enter a valid URL.'; + } else { + delete errors.url; + } + } catch (e) { + errors.url = 'Please enter a valid URL.'; + } + return errors; +}; + const AddRecommendationModal: React.FC = ({searchParams, recommendation, animate}) => { const [enterPressed, setEnterPressed] = useState(false); const modal = useModal(); @@ -41,7 +57,7 @@ const AddRecommendationModal: React.FC { const newErrors: Record = {}; - try { - const u = new URL(formState.url); - - // Check domain includes a dot - if (!u.hostname.includes('.')) { - newErrors.url = 'Please enter a valid URL.'; - } - } catch (e) { - newErrors.url = 'Please enter a valid URL.'; - } + validateUrl(newErrors, formState.url); // If we have errors: close direct submit view if (showLoadingView) { @@ -225,7 +232,13 @@ const AddRecommendationModal: React.FC updateForm(state => ({...state, url: doFormatUrl(formState.url)}))} + onBlur={() => { + const url = doFormatUrl(formState.url); + setErrors( + validateUrl(errors, url) + ); + updateForm(state => ({...state, url: url})); + }} onChange={(e) => { clearError?.('url'); updateForm(state => ({...state, url: e.target.value})); diff --git a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModalConfirm.tsx b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModalConfirm.tsx index 04163a572d..3508983eb5 100644 --- a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModalConfirm.tsx +++ b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModalConfirm.tsx @@ -2,7 +2,7 @@ import AddRecommendationModal from './AddRecommendationModal'; import Modal from '../../../../admin-x-ds/global/modal/Modal'; import NiceModal, {useModal} from '@ebay/nice-modal-react'; import React from 'react'; -import RecommendationReasonForm from './RecommendationReasonForm'; +import RecommendationReasonForm, {validateReasonForm} from './RecommendationReasonForm'; import useForm from '../../../../hooks/useForm'; import useHandleError from '../../../../utils/api/handleError'; import useRouting from '../../../../hooks/useRouting'; @@ -20,12 +20,12 @@ const AddRecommendationModalConfirm: React.FC = ({r const {mutateAsync: addRecommendation} = useAddRecommendation(); const handleError = useHandleError(); - const {formState, updateForm, handleSave, saveState, errors, clearError} = useForm({ + const {formState, updateForm, handleSave, saveState, errors, clearError, setErrors} = useForm({ initialState: { ...recommendation }, - onSave: async () => { - await addRecommendation(formState); + onSave: async (state) => { + await addRecommendation(state); modal.remove(); showToast({ message: 'Successfully added a recommendation', @@ -34,15 +34,16 @@ const AddRecommendationModalConfirm: React.FC = ({r updateRoute('recommendations'); }, onSaveError: handleError, - onValidate: () => { - const newErrors: Record = {}; - if (!formState.title) { - newErrors.title = 'Title is required'; + onValidate: (state) => { + const newErrors = validateReasonForm(state); + + if (Object.keys(newErrors).length !== 0) { + showToast({ + type: 'pageError', + message: 'Can\'t add recommendation, please double check that you\'ve filled all mandatory fields correctly.' + }); } - if (formState.reason && formState.reason.length > 200) { - newErrors.reason = 'Description cannot be longer than 200 characters'; - } return newErrors; } }); @@ -122,7 +123,7 @@ const AddRecommendationModalConfirm: React.FC = ({r } }} > - + ; }; diff --git a/apps/admin-x-settings/src/components/settings/site/recommendations/EditRecommendationModal.tsx b/apps/admin-x-settings/src/components/settings/site/recommendations/EditRecommendationModal.tsx index 35b9be056b..41d2e26bd6 100644 --- a/apps/admin-x-settings/src/components/settings/site/recommendations/EditRecommendationModal.tsx +++ b/apps/admin-x-settings/src/components/settings/site/recommendations/EditRecommendationModal.tsx @@ -2,7 +2,7 @@ import ConfirmationModal from '../../../../admin-x-ds/global/modal/ConfirmationM import Modal from '../../../../admin-x-ds/global/modal/Modal'; import NiceModal, {useModal} from '@ebay/nice-modal-react'; import React from 'react'; -import RecommendationReasonForm from './RecommendationReasonForm'; +import RecommendationReasonForm, {validateReasonForm} from './RecommendationReasonForm'; import useForm from '../../../../hooks/useForm'; import useHandleError from '../../../../utils/api/handleError'; import useRouting from '../../../../hooks/useRouting'; @@ -22,25 +22,24 @@ const EditRecommendationModal: React.FC { - await editRecommendation(formState); + onSave: async (state) => { + await editRecommendation(state); modal.remove(); updateRoute('recommendations'); }, onSaveError: handleError, - onValidate: () => { - const newErrors: Record = {}; + onValidate: (state) => { + const newErrors = validateReasonForm(state); - if (!formState.title) { - newErrors.title = 'Title is required'; - } - - if (formState.reason && formState.reason.length > 200) { - newErrors.reason = 'Description cannot be longer than 200 characters'; + if (Object.keys(newErrors).length !== 0) { + showToast({ + type: 'pageError', + message: 'Can\'t edit recommendation, please double check that you\'ve filled all mandatory fields correctly.' + }); } return newErrors; @@ -120,7 +119,7 @@ const EditRecommendationModal: React.FC - + ; }; diff --git a/apps/admin-x-settings/src/components/settings/site/recommendations/RecommendationReasonForm.tsx b/apps/admin-x-settings/src/components/settings/site/recommendations/RecommendationReasonForm.tsx index c1400f4a11..04cc7e3c17 100644 --- a/apps/admin-x-settings/src/components/settings/site/recommendations/RecommendationReasonForm.tsx +++ b/apps/admin-x-settings/src/components/settings/site/recommendations/RecommendationReasonForm.tsx @@ -14,12 +14,56 @@ interface Props { formState: T, errors: ErrorMessages, updateForm: (fn: (state: T) => T) => void, - clearError?: (key: keyof ErrorMessages) => void + clearError?: (key: keyof ErrorMessages) => void, + setErrors: (errors: ErrorMessages) => void } -const RecommendationReasonForm: React.FC> = ({showURL, formState, updateForm, errors, clearError}) => { +export const validateReasonFormField = function (errors: ErrorMessages, field: 'title'|'reason', value: string|null) { + const cloned = {...errors}; + switch (field) { + case 'title': + if (!value) { + cloned.title = 'Title is required'; + } else { + delete cloned.title; + } + break; + case 'reason': + if (value && value.length > 200) { + cloned.reason = 'Description cannot be longer than 200 characters'; + } else { + delete cloned.reason; + } + break; + default: + // Will throw a compile error if we forget to add a case for a field + const f: never = field; + throw new Error(`Unknown field ${f}`); + } + return cloned; +}; + +export const validateReasonForm = function (formState: EditOrAddRecommendation) { + let newErrors: ErrorMessages = {}; + newErrors = validateReasonFormField(newErrors, 'title', formState.title); + newErrors = validateReasonFormField(newErrors, 'reason', formState.reason); + return newErrors; +}; + +const RecommendationReasonForm: React.FC> = ({showURL, formState, updateForm, errors, clearError, setErrors}) => { const [reasonLength, setReasonLength] = React.useState(formState?.reason?.length || 0); const reasonLengthColor = reasonLength > 200 ? 'text-red' : 'text-green'; + + // Do an intial validation on mounting + const didValidate = React.useRef(false); + React.useEffect(() => { + if (didValidate.current) { + return; + } + didValidate.current = true; + setErrors(validateReasonForm(formState)); + }, [formState, setErrors]); + return
setErrors( + validateReasonFormField(errors, 'title', formState.title) + )} onChange={(e) => { clearError?.('title'); updateForm(state => ({...state, title: e.target.value})); @@ -71,10 +118,14 @@ const RecommendationReasonForm: React.FCMax: 200 characters. You’ve used {reasonLength}} + // Note: we don't show the error text here, because errors are related to the character count + hint={<>Max: 200 characters. You’ve used {reasonLength}} rows={4} title="Short description" value={formState.reason ?? ''} + onBlur={() => setErrors( + validateReasonFormField(errors, 'reason', formState.reason) + )} onChange={(e) => { clearError?.('reason'); setReasonLength(e.target.value.length); diff --git a/apps/admin-x-settings/src/hooks/useForm.ts b/apps/admin-x-settings/src/hooks/useForm.ts index 2bc0c8be3f..a566dd3b5d 100644 --- a/apps/admin-x-settings/src/hooks/useForm.ts +++ b/apps/admin-x-settings/src/hooks/useForm.ts @@ -29,13 +29,14 @@ export interface FormHook { clearError: (field: string) => void; isValid: boolean; errors: ErrorMessages; + setErrors: (errors: ErrorMessages) => void; } const useForm = ({initialState, onSave, onSaveError, onValidate}: { initialState: State, - onSave: () => void | Promise + onSave: (state: State) => void | Promise onSaveError?: (error: unknown) => void | Promise - onValidate?: () => ErrorMessages + onValidate?: (state: State) => ErrorMessages }): FormHook => { const [formState, setFormState] = useState(initialState); const [saveState, setSaveState] = useState(''); @@ -52,37 +53,43 @@ const useForm = ({initialState, onSave, onSaveError, onValidate}: { const isValid = (errs: ErrorMessages) => Object.values(errs).filter(Boolean).length === 0; - const validate = () => { - if (!onValidate) { - return true; - } + const validate = useCallback( + () => { + if (!onValidate) { + return true; + } - const newErrors = onValidate(); - setErrors(newErrors); - return isValid(newErrors); - }; + const newErrors = onValidate(formState); + setErrors(newErrors); + return isValid(newErrors); + }, + [formState, onValidate] + ); // function to save the changed settings via API - const handleSave = async (options: {force?: boolean} = {}) => { - if (!validate()) { - return false; - } + const handleSave = useCallback( + async (options: {force?: boolean} = {}) => { + if (!validate()) { + return false; + } - if (saveState !== 'unsaved' && !options.force) { - return true; - } + if (saveState !== 'unsaved' && !options.force) { + return true; + } - setSaveState('saving'); - try { - await onSave(); - setSaveState('saved'); - return true; - } catch (e) { - await onSaveError?.(e); - setSaveState('unsaved'); - throw e; - } - }; + setSaveState('saving'); + try { + await onSave(formState); + setSaveState('saved'); + return true; + } catch (e) { + await onSaveError?.(e); + setSaveState('unsaved'); + throw e; + } + }, + [formState, saveState, onSave, onSaveError, validate] + ); const updateForm = useCallback((updater: (state: State) => State) => { setFormState(updater); @@ -104,7 +111,8 @@ const useForm = ({initialState, onSave, onSaveError, onValidate}: { clearError: (field: string) => { setErrors(state => ({...state, [field]: ''})); }, - errors + errors, + setErrors }; };