From 48b90e42535ffb6a1d19175d23ee39d862652e9c Mon Sep 17 00:00:00 2001 From: Jono M Date: Mon, 30 Oct 2023 17:57:19 +0000 Subject: [PATCH] Improved modal save handling in Admin X (#18794) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Product/issues/4060 --- ### 🤖 Generated by Copilot at 55149bc Refactored various settings forms and modals in the admin app to use the updated `useForm` hook, which simplifies the form state management, validation, and saving logic. Improved the UI and UX of the modal ok buttons by using the `okProps` object and the `saveState` from the hook. Added options to save the forms without changes and to fake the saving when unchanged. Fixed some bugs and removed unused code. --- .../integrations/CustomIntegrationModal.tsx | 18 +- .../newsletters/NewsletterDetailModal.tsx | 11 +- .../settings/general/UserDetailModal.tsx | 258 ++++++++---------- .../settings/general/users/ProfileBasics.tsx | 6 +- .../settings/general/users/ProfileDetails.tsx | 12 +- .../membership/portal/PortalModal.tsx | 15 +- .../EditRecommendationModal.tsx | 24 +- .../membership/tiers/TierDetailModal.tsx | 67 +++-- .../settings/site/AnnouncementBarModal.tsx | 9 +- .../components/settings/site/DesignModal.tsx | 18 +- .../settings/site/NavigationModal.tsx | 1 + apps/admin-x-settings/src/hooks/useForm.ts | 91 ++++-- .../src/hooks/useSettingGroup.tsx | 15 +- 13 files changed, 282 insertions(+), 263 deletions(-) diff --git a/apps/admin-x-settings/src/components/settings/advanced/integrations/CustomIntegrationModal.tsx b/apps/admin-x-settings/src/components/settings/advanced/integrations/CustomIntegrationModal.tsx index 70ae842c40..88af3f0ffc 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/integrations/CustomIntegrationModal.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/integrations/CustomIntegrationModal.tsx @@ -27,11 +27,17 @@ const CustomIntegrationModalContent: React.FC<{integration: Integration}> = ({in const {mutateAsync: uploadImage} = useUploadImage(); const handleError = useHandleError(); - const {formState, updateForm, handleSave, saveState, errors, clearError, validate} = useForm({ + const {formState, updateForm, handleSave, saveState, errors, clearError, validate, okProps} = useForm({ initialState: integration, + savingDelay: 500, + savedDelay: 500, onSave: async () => { await editIntegration(formState); }, + onSavedStateReset: () => { + modal.remove(); + updateRoute('integrations'); + }, onSaveError: handleError, onValidate: () => { const newErrors: Record = {}; @@ -82,19 +88,17 @@ const CustomIntegrationModalContent: React.FC<{integration: Integration}> = ({in afterClose={() => { updateRoute('integrations'); }} + buttonsDisabled={okProps.disabled} dirty={saveState === 'unsaved'} - okColor='black' - okLabel='Save & close' + okColor={okProps.color} + okLabel={okProps.label || 'Save & close'} size='md' testId='custom-integration-modal' title={formState.name} stickyFooter onOk={async () => { toast.remove(); - if (await handleSave()) { - modal.remove(); - updateRoute('integrations'); - } else { + if (!(await handleSave({fakeWhenUnchanged: true}))) { showToast({ type: 'pageError', message: 'Can\'t save integration, please double check that you\'ve filled all mandatory fields.' diff --git a/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx b/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx index 2163d1e09a..e3042b2a00 100644 --- a/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx +++ b/apps/admin-x-settings/src/components/settings/email/newsletters/NewsletterDetailModal.tsx @@ -438,8 +438,9 @@ const NewsletterDetailModalContent: React.FC<{newsletter: Newsletter; onlyOne: b const {updateRoute} = useRouting(); const handleError = useHandleError(); - const {formState, saveState, updateForm, setFormState, handleSave, validate, errors, clearError} = useForm({ + const {formState, saveState, updateForm, setFormState, handleSave, validate, errors, clearError, okProps} = useForm({ initialState: newsletter, + savingDelay: 500, onSave: async () => { const {newsletters, meta} = await editNewsletter(formState); if (meta?.sent_email_verification) { @@ -489,12 +490,12 @@ const NewsletterDetailModalContent: React.FC<{newsletter: Newsletter; onlyOne: b return updateRoute('newsletters')} - buttonsDisabled={saveState === 'saving'} + buttonsDisabled={okProps.disabled} cancelLabel='Close' deviceSelector={false} dirty={saveState === 'unsaved'} - okColor={saveState === 'saved' ? 'green' : 'black'} - okLabel={saveState === 'saved' ? 'Saved' : (saveState === 'saving' ? 'Saving...' : 'Save')} + okColor={okProps.color} + okLabel={okProps.label || 'Save'} preview={preview} previewBgColor={'grey'} previewToolbar={false} @@ -503,7 +504,7 @@ const NewsletterDetailModalContent: React.FC<{newsletter: Newsletter; onlyOne: b testId='newsletter-modal' title='Newsletter' onOk={async () => { - if (!(await handleSave())) { + if (!(await handleSave({fakeWhenUnchanged: true}))) { showToast({ type: 'pageError', message: 'Can\'t save newsletter, please double check that you\'ve filled all mandatory fields.' diff --git a/apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx b/apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx index e9cf1aad6f..e27f1c9cc7 100644 --- a/apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx +++ b/apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx @@ -10,9 +10,10 @@ import Modal from '../../../admin-x-ds/global/modal/Modal'; import NiceModal, {useModal} from '@ebay/nice-modal-react'; import ProfileBasics from './users/ProfileBasics'; import ProfileDetails from './users/ProfileDetails'; -import React, {useCallback, useEffect, useState} from 'react'; +import React, {useCallback, useEffect} from 'react'; import StaffToken from './users/StaffToken'; import clsx from 'clsx'; +import useForm, {ErrorMessages} from '../../../hooks/useForm'; import useHandleError from '../../../utils/api/handleError'; import usePinturaEditor from '../../../hooks/usePinturaEditor'; import useRouting from '../../../hooks/useRouting'; @@ -28,11 +29,69 @@ import {toast} from 'react-hot-toast'; import {useGlobalData} from '../../providers/GlobalDataProvider'; import {validateFacebookUrl, validateTwitterUrl} from '../../../utils/socialUrls'; +const validators: Record) => string> = { + name: ({name}) => { + let error = ''; + + if (!name) { + error = 'Please enter a name'; + } + + if (name && name.length > 191) { + error = 'Name is too long'; + } + + return error; + }, + email: ({email}) => { + const valid = validator.isEmail(email || ''); + return valid ? '' : 'Please enter a valid email address'; + }, + url: ({url}) => { + const valid = !url || validator.isURL(url); + return valid ? '' : 'Please enter a valid URL'; + }, + bio: ({bio}) => { + const valid = !bio || bio.length <= 200; + return valid ? '' : 'Bio is too long'; + }, + location: ({location}) => { + const valid = !location || location.length <= 150; + return valid ? '' : 'Location is too long'; + }, + website: ({website}) => { + const valid = !website || (validator.isURL(website) && website.length <= 2000); + return valid ? '' : 'Website is not a valid url'; + }, + facebook: ({facebook}) => { + try { + validateFacebookUrl(facebook || ''); + return ''; + } catch (e) { + if (e instanceof Error) { + return e.message; + } + return ''; + } + }, + twitter: ({twitter}) => { + try { + validateTwitterUrl(twitter || ''); + return ''; + } catch (e) { + if (e instanceof Error) { + return e.message; + } + return ''; + } + } +}; + export interface UserDetailProps { user: User; setUserData: (user: User) => void; errors: {[key in keyof User]?: string}; - validators: Record) => boolean>; + validateField: (key: K, value: User[K]) => boolean; clearError: (key: keyof User) => void; } @@ -47,15 +106,39 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { const {updateRoute} = useRouting(); const {ownerUser} = useStaffUsers(); const {currentUser} = useGlobalData(); - const [userData, _setUserData] = useState(user); - const [saveState, setSaveState] = useState<'' | 'unsaved' | 'saving' | 'saved'>(''); - const [errors, setErrors] = useState({}); - - const clearError = (key: keyof User) => setErrors(errs => ({...errs, [key]: undefined})); - - const setUserData = (newUserData: User | ((current: User) => User)) => { - _setUserData(newUserData); - setSaveState('unsaved'); + const handleError = useHandleError(); + const {formState, setFormState, saveState, handleSave, updateForm, errors, setErrors, clearError, okProps} = useForm({ + initialState: user, + savingDelay: 500, + savedDelay: 500, + onValidate: (values) => { + return Object.entries(validators).reduce((newErrors, [key, validate]) => { + const error = validate(values); + if (error) { + newErrors[key] = error; + } + return newErrors; + }, {}); + }, + onSave: async (values) => { + await updateUser?.(values); + }, + onSavedStateReset: () => { + mainModal.remove(); + navigateOnClose(); + }, + onSaveError: handleError + }); + const setUserData = (newData: User) => updateForm(() => newData); + const validateField = (key: K, value: User[K]) => { + const error = validators[key]?.({[key]: value}); + if (error) { + setErrors({...errors, [key]: error}); + return false; + } else { + clearError(key); + return true; + } }; const mainModal = useModal(); @@ -64,7 +147,6 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { const {mutateAsync: deleteUser} = useDeleteUser(); const {mutateAsync: makeOwner} = useMakeOwner(); const limiter = useLimiter(); - const handleError = useHandleError(); // Pintura integration const {settings} = useGlobalData(); @@ -86,15 +168,6 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { } }, [currentUser, updateRoute]); - useEffect(() => { - if (saveState === 'saved') { - setTimeout(() => { - mainModal.remove(); - navigateOnClose(); - }, 300); - } - }, [mainModal, navigateOnClose, saveState, updateRoute]); - const confirmSuspend = async (_user: User) => { if (_user.status === 'inactive' && _user.roles[0].name !== 'Contributor') { try { @@ -133,7 +206,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { }; try { await updateUser(updatedUserData); - setUserData(updatedUserData); + setFormState(() => updatedUserData); modal?.remove(); showToast({ message: _user.status === 'inactive' ? 'User un-suspended' : 'User suspended', @@ -201,12 +274,12 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { switch (image) { case 'cover_image': - setUserData?.((_user) => { + updateForm((_user) => { return {..._user, cover_image: imageUrl}; }); break; case 'profile_image': - setUserData?.((_user) => { + updateForm((_user) => { return {..._user, profile_image: imageUrl}; }); break; @@ -219,12 +292,12 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { const handleImageDelete = (image: string) => { switch (image) { case 'cover_image': - setUserData?.((_user) => { + updateForm((_user) => { return {..._user, cover_image: ''}; }); break; case 'profile_image': - setUserData?.((_user) => { + updateForm((_user) => { return {..._user, profile_image: ''}; }); break; @@ -234,7 +307,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { const showMenu = hasAdminAccess(currentUser) || (isEditorUser(currentUser) && isAuthorOrContributor(user)); let menuItems: MenuItem[] = []; - if (isOwnerUser(currentUser) && isAdminUser(userData) && userData.status !== 'inactive') { + if (isOwnerUser(currentUser) && isAdminUser(formState) && formState.status !== 'inactive') { menuItems.push({ id: 'make-owner', label: 'Make owner', @@ -242,11 +315,11 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { }); } - if (userData.id !== currentUser.id && ( + if (formState.id !== currentUser.id && ( (hasAdminAccess(currentUser) && !isOwnerUser(user)) || (isEditorUser(currentUser) && isAuthorOrContributor(user)) )) { - let suspendUserLabel = userData.status === 'inactive' ? 'Un-suspend user' : 'Suspend user'; + let suspendUserLabel = formState.status === 'inactive' ? 'Un-suspend user' : 'Suspend user'; menuItems.push({ id: 'delete-user', @@ -258,7 +331,7 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { id: 'suspend-user', label: suspendUserLabel, onClick: () => { - confirmSuspend(userData); + confirmSuspend(formState); } }); } @@ -268,18 +341,10 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { label: 'View user activity', onClick: () => { mainModal.remove(); - updateRoute(`history/view/${userData.id}`); + updateRoute(`history/view/${formState.id}`); } }); - let okLabel = saveState === 'saved' ? 'Saved' : 'Save & close'; - - if (saveState === 'saving') { - okLabel = 'Saving...'; - } else if (saveState === 'saved') { - okLabel = 'Saved'; - } - const coverEditButtonBaseClasses = 'bg-[rgba(0,0,0,0.75)] rounded text-sm text-white flex items-center justify-center px-3 h-8 opacity-80 hover:opacity-100 transition-all cursor-pointer font-medium nowrap'; const fileUploadButtonClasses = clsx( @@ -294,122 +359,35 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { coverEditButtonBaseClasses ); - const suspendedText = userData.status === 'inactive' ? ' (Suspended)' : ''; - - const validators: Record) => boolean> = { - name: ({name}) => { - let error = ''; - - if (!name) { - error = 'Please enter a name'; - } - - if (name && name.length > 191) { - error = 'Name is too long'; - } - - setErrors?.((_errors) => { - return {..._errors, name: error}; - }); - return !error; - }, - email: ({email}) => { - const valid = validator.isEmail(email || ''); - setErrors?.((_errors) => { - return {..._errors, email: valid ? '' : 'Please enter a valid email address'}; - }); - return valid; - }, - url: ({url}) => { - const valid = !url || validator.isURL(url); - setErrors?.((_errors) => { - return {..._errors, url: valid ? '' : 'Please enter a valid URL'}; - }); - return valid; - }, - bio: ({bio}) => { - const valid = !bio || bio.length <= 200; - setErrors?.((_errors) => { - return {..._errors, bio: valid ? '' : 'Bio is too long'}; - }); - return valid; - }, - location: ({location}) => { - const valid = !location || location.length <= 150; - setErrors?.((_errors) => { - return {..._errors, location: valid ? '' : 'Location is too long'}; - }); - return valid; - }, - website: ({website}) => { - const valid = !website || (validator.isURL(website) && website.length <= 2000); - setErrors?.((_errors) => { - return {..._errors, website: valid ? '' : 'Website is not a valid url'}; - }); - return valid; - }, - facebook: ({facebook}) => { - try { - validateFacebookUrl(facebook || ''); - return true; - } catch (e) { - if (e instanceof Error) { - const message = e.message; - setErrors?.(_errors => ({..._errors, facebook: message})); - } - return false; - } - }, - twitter: ({twitter}) => { - try { - validateTwitterUrl(twitter || ''); - return true; - } catch (e) { - if (e instanceof Error) { - const message = e.message; - setErrors?.(_errors => ({..._errors, twitter: message})); - } - return false; - } - } - }; + const suspendedText = formState.status === 'inactive' ? ' (Suspended)' : ''; return ( { - setSaveState('saving'); - let isValid = true; - if (Object.values(validators).map(validate => validate(userData)).includes(false)) { - isValid = false; - } - toast.remove(); - if (!isValid) { + if (!(await handleSave({fakeWhenUnchanged: true}))) { showToast({ type: 'pageError', message: 'Can\'t save user, please double check that you\'ve filled all mandatory fields.' }); - setSaveState(''); - return; } - - await updateUser?.(userData); - setSaveState('saved'); }} >
@@ -422,12 +400,12 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { id='avatar' imageClassName='w-full h-full object-cover rounded-full shrink-0' imageContainerClassName='relative group bg-cover bg-center -ml-2 h-[80px] w-[80px] shrink-0' - imageURL={userData.profile_image} + imageURL={formState.profile_image} pintura={ { isEnabled: editor.isEnabled, openEditor: async () => editor.openEditor({ - image: userData.profile_image || '', + image: formState.profile_image || '', handleSave: async (file:File) => { handleImageUpload('profile_image', file); } @@ -460,12 +438,12 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => { fileUploadClassName={fileUploadButtonClasses} id='cover-image' imageClassName='hidden' - imageURL={userData.cover_image || ''} + imageURL={formState.cover_image || ''} pintura={ { isEnabled: editor.isEnabled, openEditor: async () => editor.openEditor({ - image: userData.cover_image || '', + image: formState.cover_image || '', handleSave: async (file:File) => { handleImageUpload('cover_image', file); } @@ -487,13 +465,13 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
- +
- + {user.id === currentUser.id && }
- - + +
diff --git a/apps/admin-x-settings/src/components/settings/general/users/ProfileBasics.tsx b/apps/admin-x-settings/src/components/settings/general/users/ProfileBasics.tsx index d33c0a8b46..b0f035ea2b 100644 --- a/apps/admin-x-settings/src/components/settings/general/users/ProfileBasics.tsx +++ b/apps/admin-x-settings/src/components/settings/general/users/ProfileBasics.tsx @@ -7,7 +7,7 @@ import {UserDetailProps} from '../UserDetailModal'; import {hasAdminAccess} from '../../../../api/users'; import {useGlobalData} from '../../../providers/GlobalDataProvider'; -const BasicInputs: React.FC = ({errors, validators, clearError, user, setUserData}) => { +const BasicInputs: React.FC = ({errors, validateField, clearError, user, setUserData}) => { const {currentUser} = useGlobalData(); return ( @@ -18,7 +18,7 @@ const BasicInputs: React.FC = ({errors, validators, clearError, title="Full name" value={user.name} onBlur={(e) => { - validators.name({name: e.target.value}); + validateField('name', e.target.value); }} onChange={(e) => { setUserData({...user, name: e.target.value}); @@ -31,7 +31,7 @@ const BasicInputs: React.FC = ({errors, validators, clearError, title="Email" value={user.email} onBlur={(e) => { - validators.email({email: e.target.value}); + validateField('email', e.target.value); }} onChange={(e) => { setUserData({...user, email: e.target.value}); diff --git a/apps/admin-x-settings/src/components/settings/general/users/ProfileDetails.tsx b/apps/admin-x-settings/src/components/settings/general/users/ProfileDetails.tsx index 9c33696009..0d30f41726 100644 --- a/apps/admin-x-settings/src/components/settings/general/users/ProfileDetails.tsx +++ b/apps/admin-x-settings/src/components/settings/general/users/ProfileDetails.tsx @@ -7,7 +7,7 @@ import {UserDetailProps} from '../UserDetailModal'; import {facebookHandleToUrl, facebookUrlToHandle, twitterHandleToUrl, twitterUrlToHandle, validateFacebookUrl, validateTwitterUrl} from '../../../../utils/socialUrls'; import {useState} from 'react'; -export const DetailsInputs: React.FC = ({errors, clearError, validators, user, setUserData}) => { +export const DetailsInputs: React.FC = ({errors, clearError, validateField, user, setUserData}) => { const [facebookUrl, setFacebookUrl] = useState(user.facebook ? facebookHandleToUrl(user.facebook) : ''); const [twitterUrl, setTwitterUrl] = useState(user.twitter ? twitterHandleToUrl(user.twitter) : ''); @@ -19,7 +19,7 @@ export const DetailsInputs: React.FC = ({errors, clearError, va title="Location" value={user.location || ''} onBlur={(e) => { - validators.location({location: e.target.value}); + validateField('location', e.target.value); }} onChange={(e) => { setUserData({...user, location: e.target.value}); @@ -31,7 +31,7 @@ export const DetailsInputs: React.FC = ({errors, clearError, va title="Website" value={user.website || ''} onBlur={(e) => { - validators.url({url: e.target.value}); + validateField('url', e.target.value); }} onChange={(e) => { setUserData({...user, website: e.target.value}); @@ -43,7 +43,7 @@ export const DetailsInputs: React.FC = ({errors, clearError, va title="Facebook profile" value={facebookUrl} onBlur={(e) => { - if (validators.facebook({facebook: e.target.value})) { + if (validateField('facebook', e.target.value)) { const url = validateFacebookUrl(e.target.value); setFacebookUrl(url); setUserData({...user, facebook: facebookUrlToHandle(url)}); @@ -59,7 +59,7 @@ export const DetailsInputs: React.FC = ({errors, clearError, va title="X (formerly Twitter) profile" value={twitterUrl} onBlur={(e) => { - if (validators.twitter({twitter: e.target.value})) { + if (validateField('twitter', e.target.value)) { const url = validateTwitterUrl(e.target.value); setTwitterUrl(url); setUserData({...user, twitter: twitterUrlToHandle(url)}); @@ -75,7 +75,7 @@ export const DetailsInputs: React.FC = ({errors, clearError, va title="Bio" value={user.bio || ''} onBlur={(e) => { - validators.bio({bio: e.target.value}); + validateField('bio', e.target.value); }} onChange={(e) => { setUserData({...user, bio: e.target.value}); diff --git a/apps/admin-x-settings/src/components/settings/membership/portal/PortalModal.tsx b/apps/admin-x-settings/src/components/settings/membership/portal/PortalModal.tsx index ce0718b20e..250855c212 100644 --- a/apps/admin-x-settings/src/components/settings/membership/portal/PortalModal.tsx +++ b/apps/admin-x-settings/src/components/settings/membership/portal/PortalModal.tsx @@ -115,12 +115,14 @@ const PortalModal: React.FC = () => { } }, [handleError, verifyEmail, verifyToken]); - const {formState, setFormState, saveState, handleSave, updateForm} = useForm({ + const {formState, setFormState, saveState, handleSave, updateForm, okProps} = useForm({ initialState: { settings: settings as Dirtyable[], tiers: allTiers as Dirtyable[] || [] }, + savingDelay: 500, + onSave: async () => { await Promise.all(formState.tiers.filter(({dirty}) => dirty).map(tier => editTier(tier))); setFormState(state => ({...state, tiers: formState.tiers.map(tier => ({...tier, dirty: false}))})); @@ -204,22 +206,17 @@ const PortalModal: React.FC = () => { {id: 'account', title: 'Account page'}, {id: 'links', title: 'Links'} ]; - let okLabel = 'Save'; - if (saveState === 'saving') { - okLabel = 'Saving...'; - } else if (saveState === 'saved') { - okLabel = 'Saved'; - } return { updateRoute('portal'); }} + buttonsDisabled={okProps.disabled} cancelLabel='Close' deviceSelector={false} dirty={saveState === 'unsaved'} - okColor={saveState === 'saved' ? 'green' : 'black'} - okLabel={okLabel} + okColor={okProps.color} + okLabel={okProps.label || 'Save'} preview={preview} previewBgColor={selectedPreviewTab === 'links' ? 'white' : 'greygradient'} previewToolbarTabs={previewTabs} diff --git a/apps/admin-x-settings/src/components/settings/membership/recommendations/EditRecommendationModal.tsx b/apps/admin-x-settings/src/components/settings/membership/recommendations/EditRecommendationModal.tsx index 4d5feb3597..d44431cb80 100644 --- a/apps/admin-x-settings/src/components/settings/membership/recommendations/EditRecommendationModal.tsx +++ b/apps/admin-x-settings/src/components/settings/membership/recommendations/EditRecommendationModal.tsx @@ -22,12 +22,16 @@ const EditRecommendationModal: React.FC { await editRecommendation(state); + }, + onSavedStateReset: () => { modal.remove(); updateRoute('recommendations'); }, @@ -46,14 +50,6 @@ const EditRecommendationModal: React.FC { - if (saveState === 'saving') { - // Already saving - return; - } - dismissAllToasts(); try { await handleSave({force: true}); diff --git a/apps/admin-x-settings/src/components/settings/membership/tiers/TierDetailModal.tsx b/apps/admin-x-settings/src/components/settings/membership/tiers/TierDetailModal.tsx index 30640a871b..ec72400212 100644 --- a/apps/admin-x-settings/src/components/settings/membership/tiers/TierDetailModal.tsx +++ b/apps/admin-x-settings/src/components/settings/membership/tiers/TierDetailModal.tsx @@ -6,14 +6,14 @@ import Heading from '../../../../admin-x-ds/global/Heading'; import Icon from '../../../../admin-x-ds/global/Icon'; import Modal from '../../../../admin-x-ds/global/modal/Modal'; import NiceModal, {useModal} from '@ebay/nice-modal-react'; -import React, {useEffect, useRef, useState} from 'react'; +import React, {useEffect, useRef} from 'react'; import Select from '../../../../admin-x-ds/global/form/Select'; import SortableList from '../../../../admin-x-ds/global/SortableList'; import TextField from '../../../../admin-x-ds/global/form/TextField'; import TierDetailPreview from './TierDetailPreview'; import Toggle from '../../../../admin-x-ds/global/form/Toggle'; import URLTextField from '../../../../admin-x-ds/global/form/URLTextField'; -import useForm from '../../../../hooks/useForm'; +import useForm, {ErrorMessages} from '../../../../hooks/useForm'; import useHandleError from '../../../../utils/api/handleError'; import useRouting from '../../../../hooks/useRouting'; import useSettingGroup from '../../../../hooks/useSettingGroup'; @@ -41,14 +41,13 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { const {localSettings, siteData} = useSettingGroup(); const siteTitle = getSettingValues(localSettings, ['title']) as string[]; - const [errors, setErrors] = useState<{ [key in keyof Tier]?: string }>({}); // eslint-disable-line no-unused-vars - - const setError = (field: keyof Tier, error: string | undefined) => { - setErrors(errs => ({...errs, [field]: error})); - return error; + const validators: {[key in keyof Tier]?: () => string | undefined} = { + name: () => (formState.name ? undefined : 'You must specify a name'), + monthly_price: () => (formState.type !== 'free' ? validateCurrencyAmount(formState.monthly_price || 0, formState.currency, {allowZero: false}) : undefined), + yearly_price: () => (formState.type !== 'free' ? validateCurrencyAmount(formState.yearly_price || 0, formState.currency, {allowZero: false}) : undefined) }; - const {formState, saveState, updateForm, handleSave} = useForm({ + const {formState, saveState, updateForm, handleSave, errors, setErrors, clearError, okProps} = useForm({ initialState: { ...(tier || {}), trial_days: tier?.trial_days?.toString() || '', @@ -56,6 +55,17 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { visibility: tier?.visibility || 'none', welcome_page_url: tier?.welcome_page_url || null }, + savingDelay: 500, + savedDelay: 500, + onValidate: () => { + const newErrors: ErrorMessages = {}; + + Object.entries(validators).forEach(([key, validator]) => { + newErrors[key as keyof Tier] = validator?.(); + }); + + return newErrors; + }, onSave: async () => { const {trial_days: trialDays, currency, ...rest} = formState; const values: Partial = rest; @@ -72,16 +82,22 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { } else { await createTier(values); } - + }, + onSavedStateReset: () => { modal.remove(); + updateRoute('tiers'); }, onSaveError: handleError }); - const validators = { - name: () => setError('name', formState.name ? undefined : 'You must specify a name'), - monthly_price: () => formState.type !== 'free' && setError('monthly_price', validateCurrencyAmount(formState.monthly_price || 0, formState.currency, {allowZero: false})), - yearly_price: () => formState.type !== 'free' && setError('yearly_price', validateCurrencyAmount(formState.yearly_price || 0, formState.currency, {allowZero: false})) + const validateField = (key: string) => { + const error = validators[key as keyof Tier]?.(); + + if (error) { + setErrors({...errors, [key]: error}); + } else { + clearError(key); + } }; const benefits = useSortableIndexedList({ @@ -105,8 +121,8 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { const didInitialRender = useRef(false); useEffect(() => { if (didInitialRender.current) { - validators.monthly_price(); - validators.yearly_price(); + validators.monthly_price?.(); + validators.yearly_price?.(); } didInitialRender.current = true; @@ -164,9 +180,11 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { afterClose={() => { updateRoute('tiers'); }} + buttonsDisabled={okProps.disabled} dirty={saveState === 'unsaved'} leftButtonProps={leftButtonProps} - okLabel='Save & close' + okColor={okProps.color} + okLabel={okProps.label || 'Save & close'} size='lg' testId='tier-detail-modal' title={(tier ? (tier.active ? 'Edit tier' : 'Edit archived tier') : 'New tier')} @@ -174,22 +192,13 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { onOk={async () => { toast.remove(); - if (Object.values(validators).filter(validator => validator()).length) { + if (!await handleSave({fakeWhenUnchanged: true})) { showToast({ type: 'pageError', message: 'Can\'t save tier, please double check that you\'ve filled all mandatory fields.' }); return; } - - if (saveState !== 'unsaved') { - updateRoute('tiers'); - modal.remove(); - } - - if (await handleSave()) { - updateRoute('tiers'); - } }} >
@@ -203,7 +212,7 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => { title='Name' value={formState.name || ''} autoFocus - onBlur={() => validators.name()} + onBlur={() => validateField('name')} onChange={e => updateForm(state => ({...state, name: e.target.value}))} />} = ({tier}) => { title='Monthly price' valueInCents={formState.monthly_price || ''} hideTitle - onBlur={() => validators.monthly_price()} + onBlur={() => validateField('monthly_price')} onChange={price => updateForm(state => ({...state, monthly_price: price}))} /> = ({tier}) => { title='Yearly price' valueInCents={formState.yearly_price || ''} hideTitle - onBlur={() => validators.yearly_price()} + onBlur={() => validateField('yearly_price')} onChange={price => updateForm(state => ({...state, yearly_price: price}))} />
diff --git a/apps/admin-x-settings/src/components/settings/site/AnnouncementBarModal.tsx b/apps/admin-x-settings/src/components/settings/site/AnnouncementBarModal.tsx index abbb19cecd..438ce609f4 100644 --- a/apps/admin-x-settings/src/components/settings/site/AnnouncementBarModal.tsx +++ b/apps/admin-x-settings/src/components/settings/site/AnnouncementBarModal.tsx @@ -115,7 +115,7 @@ const Sidebar: React.FC = ({ const AnnouncementBarModal: React.FC = () => { const {siteData} = useGlobalData(); - const {localSettings, updateSetting, handleSave} = useSettingGroup(); + const {localSettings, updateSetting, handleSave, okProps} = useSettingGroup({savingDelay: 500}); const [announcementContent] = getSettingValues(localSettings, ['announcement_content']); const [accentColor] = getSettingValues(localSettings, ['accent_color']); const [announcementBackgroundColor] = getSettingValues(localSettings, ['announcement_background']); @@ -208,10 +208,12 @@ const AnnouncementBarModal: React.FC = () => { afterClose={() => { updateRoute('announcement-bar'); }} + buttonsDisabled={okProps.disabled} cancelLabel='Close' deviceSelector={true} dirty={false} - okLabel='Save' + okColor={okProps.color} + okLabel={okProps.label || 'Save'} preview={preview} previewBgColor='greygradient' previewToolbarTabs={previewTabs} @@ -221,8 +223,7 @@ const AnnouncementBarModal: React.FC = () => { title='Announcement' titleHeadingLevel={5} onOk={async () => { - if (!(await handleSave())) { - updateRoute('announcement-bar'); + if (!(await handleSave({fakeWhenUnchanged: true}))) { showToast({ type: 'pageError', message: 'An error occurred while saving your changes. Please try again.' diff --git a/apps/admin-x-settings/src/components/settings/site/DesignModal.tsx b/apps/admin-x-settings/src/components/settings/site/DesignModal.tsx index 444de06a70..090a329bf3 100644 --- a/apps/admin-x-settings/src/components/settings/site/DesignModal.tsx +++ b/apps/admin-x-settings/src/components/settings/site/DesignModal.tsx @@ -1,6 +1,4 @@ import BrandSettings, {BrandSettingValues} from './designAndBranding/BrandSettings'; -// import Button from '../../../admin-x-ds/global/Button'; -// import ChangeThemeModal from './ThemeModal'; import Icon from '../../../admin-x-ds/global/Icon'; import React, {useEffect, useState} from 'react'; import StickyFooter from '../../../admin-x-ds/global/StickyFooter'; @@ -102,21 +100,23 @@ const DesignModal: React.FC = () => { saveState, handleSave, updateForm, - setFormState + setFormState, + okProps } = useForm({ initialState: { settings: settings as Array, themeSettings: themeSettings ? (themeSettings.custom_theme_settings as Array) : undefined }, + savingDelay: 500, onSave: async () => { if (formState.themeSettings?.some(setting => setting.dirty)) { const response = await editThemeSettings(formState.themeSettings); - updateForm(state => ({...state, themeSettings: response.custom_theme_settings})); + setFormState(state => ({...state, themeSettings: response.custom_theme_settings})); } if (formState.settings.some(setting => setting.dirty)) { const {settings: newSettings} = await editSettings(formState.settings.filter(setting => setting.dirty)); - updateForm(state => ({...state, settings: newSettings})); + setFormState(state => ({...state, settings: newSettings})); } }, onSaveError: handleError @@ -215,12 +215,12 @@ const DesignModal: React.FC = () => { afterClose={() => { updateRoute('design'); }} - buttonsDisabled={saveState === 'saving'} + buttonsDisabled={okProps.disabled} cancelLabel='Close' defaultTab='homepage' dirty={saveState === 'unsaved'} - okColor={saveState === 'saved' ? 'green' : 'black'} - okLabel={saveState === 'saved' ? 'Saved' : (saveState === 'saving' ? 'Saving...' : 'Save')} + okColor={okProps.color} + okLabel={okProps.label || 'Save'} preview={previewContent} previewToolbarTabs={previewTabs} selectedURL={selectedPreviewTab} @@ -231,7 +231,7 @@ const DesignModal: React.FC = () => { testId='design-modal' title='Design' onOk={async () => { - await handleSave(); + await handleSave({fakeWhenUnchanged: true}); }} onSelectURL={onSelectURL} />; diff --git a/apps/admin-x-settings/src/components/settings/site/NavigationModal.tsx b/apps/admin-x-settings/src/components/settings/site/NavigationModal.tsx index f62618787f..48919af276 100644 --- a/apps/admin-x-settings/src/components/settings/site/NavigationModal.tsx +++ b/apps/admin-x-settings/src/components/settings/site/NavigationModal.tsx @@ -45,6 +45,7 @@ const NavigationModal = NiceModal.create(() => { }} buttonsDisabled={saveState === 'saving'} dirty={localSettings.some(setting => setting.dirty)} + okLabel={saveState === 'saving' ? 'Saving...' : 'OK'} scrolling={true} size='lg' stickyFooter={true} diff --git a/apps/admin-x-settings/src/hooks/useForm.ts b/apps/admin-x-settings/src/hooks/useForm.ts index a566dd3b5d..a353720c7b 100644 --- a/apps/admin-x-settings/src/hooks/useForm.ts +++ b/apps/admin-x-settings/src/hooks/useForm.ts @@ -1,3 +1,4 @@ +import {ButtonColor} from '../admin-x-ds/global/Button'; import {useCallback, useEffect, useState} from 'react'; export type Dirtyable = Data & { @@ -8,13 +9,21 @@ export type SaveState = 'unsaved' | 'saving' | 'saved' | 'error' | ''; export type ErrorMessages = Record +export interface OkProps { + disabled: boolean; + color: ButtonColor; + label?: string; +} + +export type SaveHandler = (options?: {force?: boolean; fakeWhenUnchanged?: boolean}) => Promise + export interface FormHook { formState: State; saveState: SaveState; /** * Validate and save the state. Use the `force` option to save even when there are no changes made (e.g., initial state should be saveable) */ - handleSave: (options?: {force?: boolean}) => Promise; + handleSave: SaveHandler; /** * Update the form state and mark the form as dirty. Should be used in input events */ @@ -30,26 +39,33 @@ export interface FormHook { isValid: boolean; errors: ErrorMessages; setErrors: (errors: ErrorMessages) => void; + + okProps: OkProps; } -const useForm = ({initialState, onSave, onSaveError, onValidate}: { - initialState: State, - onSave: (state: State) => void | Promise - onSaveError?: (error: unknown) => void | Promise - onValidate?: (state: State) => ErrorMessages +const useForm = ({initialState, savingDelay, savedDelay = 2000, onSave, onSaveError, onSavedStateReset: onSaveCompleted, onValidate}: { + initialState: State; + savingDelay?: number; + savedDelay?: number; + onSave: (state: State) => void | Promise; + onSaveError?: (error: unknown) => void | Promise; + onSavedStateReset?: () => void; + onValidate?: (state: State) => ErrorMessages; }): FormHook => { const [formState, setFormState] = useState(initialState); const [saveState, setSaveState] = useState(''); const [errors, setErrors] = useState({}); - // Reset saved state after 2 seconds + // Reset saved state after a delay + // To prevent infinite renders, uses the value of onSaveCompleted from when the form was saved useEffect(() => { if (saveState === 'saved') { setTimeout(() => { + onSaveCompleted?.(); setSaveState(state => (state === 'saved' ? '' : state)); - }, 2000); + }, savedDelay); } - }, [saveState]); + }, [saveState, savedDelay]); // eslint-disable-line react-hooks/exhaustive-deps const isValid = (errs: ErrorMessages) => Object.values(errs).filter(Boolean).length === 0; @@ -67,35 +83,51 @@ const useForm = ({initialState, onSave, onSaveError, onValidate}: { ); // function to save the changed settings via API - const handleSave = useCallback( - async (options: {force?: boolean} = {}) => { - if (!validate()) { - return false; - } + const handleSave = useCallback(async (options = {}) => { + if (!validate()) { + return false; + } - if (saveState !== 'unsaved' && !options.force) { - return true; - } + if (saveState !== 'unsaved' && !options.force && !options.fakeWhenUnchanged) { + return true; + } - setSaveState('saving'); - try { + const timeBefore = Date.now(); + + setSaveState('saving'); + try { + if (saveState === 'unsaved' || options.force) { await onSave(formState); - setSaveState('saved'); - return true; - } catch (e) { - await onSaveError?.(e); - setSaveState('unsaved'); - throw e; } - }, - [formState, saveState, onSave, onSaveError, validate] - ); + + const duration = Date.now() - timeBefore; + if (savingDelay && duration < savingDelay) { + await new Promise((resolve) => { + setTimeout(resolve, savingDelay - duration); + }); + } + + setSaveState('saved'); + + return true; + } catch (e) { + await onSaveError?.(e); + setSaveState('unsaved'); + throw e; + } + }, [formState, saveState, savingDelay, onSave, onSaveError, validate]); const updateForm = useCallback((updater: (state: State) => State) => { setFormState(updater); setSaveState('unsaved'); }, []); + const okProps: OkProps = { + disabled: saveState === 'saving', + color: saveState === 'saved' ? 'green' : 'black', + label: saveState === 'saved' ? 'Saved' : (saveState === 'saving' ? 'Saving...' : undefined) + }; + return { formState, saveState, @@ -112,7 +144,8 @@ const useForm = ({initialState, onSave, onSaveError, onValidate}: { setErrors(state => ({...state, [field]: ''})); }, errors, - setErrors + setErrors, + okProps }; }; diff --git a/apps/admin-x-settings/src/hooks/useSettingGroup.tsx b/apps/admin-x-settings/src/hooks/useSettingGroup.tsx index 2e4dfc9ad1..e5bea15f6a 100644 --- a/apps/admin-x-settings/src/hooks/useSettingGroup.tsx +++ b/apps/admin-x-settings/src/hooks/useSettingGroup.tsx @@ -1,5 +1,5 @@ import React, {useEffect, useRef, useState} from 'react'; -import useForm, {ErrorMessages, SaveState} from './useForm'; +import useForm, {ErrorMessages, OkProps, SaveHandler, SaveState} from './useForm'; import useGlobalDirtyState from './useGlobalDirtyState'; import useHandleError from '../utils/api/handleError'; import {Setting, SettingValue, useEditSettings} from '../api/settings'; @@ -18,16 +18,17 @@ export interface SettingGroupHook { saveState: SaveState; siteData: SiteData | null; focusRef: React.RefObject; - handleSave: () => Promise; + handleSave: SaveHandler; handleCancel: () => void; updateSetting: (key: string, value: SettingValue) => void; handleEditingChange: (newState: boolean) => void; validate: () => boolean; errors: ErrorMessages; clearError: (key: string) => void; + okProps: OkProps; } -const useSettingGroup = ({onValidate}: {onValidate?: () => ErrorMessages} = {}): SettingGroupHook => { +const useSettingGroup = ({savingDelay, onValidate}: {savingDelay?: number; onValidate?: () => ErrorMessages} = {}): SettingGroupHook => { // create a ref to focus the input field const focusRef = useRef(null); @@ -37,8 +38,9 @@ const useSettingGroup = ({onValidate}: {onValidate?: () => ErrorMessages} = {}): const [isEditing, setEditing] = useState(false); - const {formState: localSettings, saveState, handleSave, updateForm, reset, validate, errors, clearError} = useForm({ + const {formState: localSettings, saveState, handleSave, updateForm, setFormState, reset, validate, errors, clearError, okProps} = useForm({ initialState: settings || [], + savingDelay, onSave: async () => { await editSettings?.(changedSettings()); }, @@ -61,7 +63,7 @@ const useSettingGroup = ({onValidate}: {onValidate?: () => ErrorMessages} = {}): // reset the local state when there's a new settings API response, unless currently editing useEffect(() => { if (!isEditing || saveState === 'saving') { - reset(); + setFormState(() => settings); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [settings]); @@ -124,7 +126,8 @@ const useSettingGroup = ({onValidate}: {onValidate?: () => ErrorMessages} = {}): handleEditingChange, validate, errors, - clearError + clearError, + okProps }; };