From fca8941740bc60bf26d85fbdb300f4edfcca7074 Mon Sep 17 00:00:00 2001 From: Peter Zimon Date: Mon, 1 Jul 2024 17:29:53 +0200 Subject: [PATCH] Updated integration modals buttons (#20502) DES-27 Updated buttons in integrations from [Cancel] and [Save & close] to [Close] and [Save] to be consistent with the rest of the Settings UI. --- .../src/global/modal/Modal.tsx | 4 +- .../advanced/integrations/AmpModal.tsx | 26 ++++++++---- .../integrations/CustomIntegrationModal.tsx | 4 +- .../integrations/FirstPromoterModal.tsx | 28 +++++++++---- .../advanced/integrations/PinturaModal.tsx | 42 +++++++++++++------ .../advanced/integrations/SlackModal.tsx | 20 ++++----- .../advanced/integrations/UnsplashModal.tsx | 42 +++++++++++++------ .../advanced/integrations/amp.test.ts | 2 +- .../advanced/integrations/custom.test.ts | 5 ++- .../integrations/firstPromoter.test.ts | 2 +- .../advanced/integrations/slack.test.ts | 9 ++-- .../advanced/integrations/unsplash.test.ts | 2 + 12 files changed, 125 insertions(+), 61 deletions(-) diff --git a/apps/admin-x-design-system/src/global/modal/Modal.tsx b/apps/admin-x-design-system/src/global/modal/Modal.tsx index 2377c3de3f..e959a3c89f 100644 --- a/apps/admin-x-design-system/src/global/modal/Modal.tsx +++ b/apps/admin-x-design-system/src/global/modal/Modal.tsx @@ -27,6 +27,7 @@ export interface ModalProps { cancelLabel?: string; leftButtonProps?: ButtonProps; buttonsDisabled?: boolean; + okDisabled?: boolean; footer?: boolean | React.ReactNode; header?: boolean; padding?: boolean; @@ -62,6 +63,7 @@ const Modal: React.FC = ({ header, leftButtonProps, buttonsDisabled, + okDisabled, padding = true, onOk, okColor = 'black', @@ -179,7 +181,7 @@ const Modal: React.FC = ({ color: okColor, className: 'min-w-[80px]', onClick: onOk, - disabled: buttonsDisabled, + disabled: buttonsDisabled || okDisabled, loading: okLoading }); } diff --git a/apps/admin-x-settings/src/components/settings/advanced/integrations/AmpModal.tsx b/apps/admin-x-settings/src/components/settings/advanced/integrations/AmpModal.tsx index 47348ca24c..9a7e52414e 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/integrations/AmpModal.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/integrations/AmpModal.tsx @@ -13,11 +13,11 @@ const AmpModal = NiceModal.create(() => { const {settings} = useGlobalData(); const [ampEnabled] = getSettingValues(settings, ['amp']); const [ampId] = getSettingValues(settings, ['amp_gtag_id']); - const modal = NiceModal.useModal(); - const [enabled, setEnabled] = useState(false); const [trackingId, setTrackingId] = useState(''); const {mutateAsync: editSettings} = useEditSettings(); const handleError = useHandleError(); + const [okLabel, setOkLabel] = useState('Save'); + const [enabled, setEnabled] = useState(!!ampEnabled); useEffect(() => { setEnabled(ampEnabled || false); @@ -30,26 +30,36 @@ const AmpModal = NiceModal.create(() => { {key: 'amp_gtag_id', value: trackingId} ]; try { - await editSettings(updates); + setOkLabel('Saving...'); + await Promise.all([ + editSettings(updates), + new Promise((resolve) => { + setTimeout(resolve, 1000); + }) + ]); + setOkLabel('Saved'); } catch (e) { handleError(e); + } finally { + setTimeout(() => setOkLabel('Save'), 1000); } }; + const isDirty = !(enabled === ampEnabled) || !(trackingId === ampId); + return ( { updateRoute('integrations'); }} - dirty={!(enabled === ampEnabled) || !(trackingId === ampId)} - okColor='black' - okLabel='Save & close' + cancelLabel='Close' + dirty={isDirty} + okColor={okLabel === 'Saved' ? 'green' : 'black'} + okLabel={okLabel} testId='amp-modal' title='' onOk={async () => { await handleSave(); - modal.remove(); - updateRoute('integrations'); }} > = ({in await editIntegration(formState); }, onSavedStateReset: () => { - modal.remove(); updateRoute('integrations'); }, onSaveError: handleError, @@ -82,9 +81,10 @@ const CustomIntegrationModalContent: React.FC<{integration: Integration}> = ({in updateRoute('integrations'); }} buttonsDisabled={okProps.disabled} + cancelLabel='Close' dirty={saveState === 'unsaved'} okColor={okProps.color} - okLabel={okProps.label || 'Save & close'} + okLabel={okProps.label || 'Save'} size='md' testId='custom-integration-modal' title={formState.name || 'Custom integration'} diff --git a/apps/admin-x-settings/src/components/settings/advanced/integrations/FirstPromoterModal.tsx b/apps/admin-x-settings/src/components/settings/advanced/integrations/FirstPromoterModal.tsx index c5410ec854..0efca1d1e5 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/integrations/FirstPromoterModal.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/integrations/FirstPromoterModal.tsx @@ -10,18 +10,19 @@ import {useRouting} from '@tryghost/admin-x-framework/routing'; const FirstpromoterModal = NiceModal.create(() => { const {updateRoute} = useRouting(); - const modal = NiceModal.useModal(); const {settings} = useGlobalData(); const {mutateAsync: editSettings} = useEditSettings(); const handleError = useHandleError(); const [accountId, setAccountId] = useState(''); - const [enabled, setEnabled] = useState(false); const [firstPromoterEnabled] = getSettingValues(settings, ['firstpromoter']); const [firstPromoterId] = getSettingValues(settings, ['firstpromoter_id']); + const [okLabel, setOkLabel] = useState('Save'); + const [enabled, setEnabled] = useState(!!firstPromoterEnabled); + useEffect(() => { setEnabled(firstPromoterEnabled || false); setAccountId(firstPromoterId || null); @@ -38,8 +39,20 @@ const FirstpromoterModal = NiceModal.create(() => { value: accountId } ]; - - await editSettings(updates); + try { + setOkLabel('Saving...'); + await Promise.all([ + editSettings(updates), + new Promise((resolve) => { + setTimeout(resolve, 1000); + }) + ]); + setOkLabel('Saved'); + } catch (e) { + handleError(e); + } finally { + setTimeout(() => setOkLabel('Save'), 1000); + } }; return ( @@ -47,16 +60,15 @@ const FirstpromoterModal = NiceModal.create(() => { afterClose={() => { updateRoute('integrations'); }} + cancelLabel='Close' dirty={enabled !== firstPromoterEnabled || accountId !== firstPromoterId} - okColor='black' - okLabel='Save & close' + okColor={okLabel === 'Saved' ? 'green' : 'black'} + okLabel={okLabel} testId='firstpromoter-modal' title='' onOk={async () => { try { await handleSave(); - updateRoute('integrations'); - modal.remove(); } catch (e) { handleError(e); } diff --git a/apps/admin-x-settings/src/components/settings/advanced/integrations/PinturaModal.tsx b/apps/admin-x-settings/src/components/settings/advanced/integrations/PinturaModal.tsx index 928f112a5d..f2763e1850 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/integrations/PinturaModal.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/integrations/PinturaModal.tsx @@ -12,8 +12,6 @@ import {useUploadFile} from '@tryghost/admin-x-framework/api/files'; const PinturaModal = NiceModal.create(() => { const {updateRoute} = useRouting(); - const modal = NiceModal.useModal(); - const [enabled, setEnabled] = useState(false); const [uploadingState, setUploadingState] = useState({ js: false, css: false @@ -29,6 +27,29 @@ const PinturaModal = NiceModal.create(() => { setEnabled(pinturaEnabled || false); }, [pinturaEnabled]); + const [okLabel, setOkLabel] = useState('Save'); + const [enabled, setEnabled] = useState(!!pinturaEnabled); + + const handleToggleChange = async () => { + const updates: Setting[] = [ + {key: 'pintura', value: (enabled)} + ]; + try { + setOkLabel('Saving...'); + await Promise.all([ + editSettings(updates), + new Promise((resolve) => { + setTimeout(resolve, 1000); + }) + ]); + setOkLabel('Saved'); + } catch (error) { + handleError(error); + } finally { + setTimeout(() => setOkLabel('Save'), 1000); + } + }; + const jsUploadRef = useRef(null); const cssUploadRef = useRef(null); const triggerUpload = (form: string) => { @@ -70,23 +91,20 @@ const PinturaModal = NiceModal.create(() => { } }; + const isDirty = !(enabled === pinturaEnabled); + return ( { updateRoute('integrations'); }} - cancelLabel='' - okColor='black' - okLabel='Save' + cancelLabel='Close' + dirty={isDirty} + okColor={okLabel === 'Saved' ? 'green' : 'black'} + okLabel={okLabel} testId='pintura-modal' title='' - onOk={async () => { - modal.remove(); - updateRoute('integrations'); - await editSettings([ - {key: 'pintura', value: enabled} - ]); - }} + onOk={handleToggleChange} > { const {updateRoute} = useRouting(); - const modal = NiceModal.useModal(); - const {localSettings, updateSetting, handleSave, validate, errors, clearError} = useSettingGroup({ + const {localSettings, updateSetting, handleSave, validate, errors, clearError, okProps} = useSettingGroup({ onValidate: () => { const newErrors: Record = {}; @@ -21,7 +20,8 @@ const SlackModal = NiceModal.create(() => { } return newErrors; - } + }, + savingDelay: 500 }); const [slackUrl, slackUsername] = getSettingValues(localSettings, ['slack_url', 'slack_username']); @@ -38,22 +38,22 @@ const SlackModal = NiceModal.create(() => { } }; + const isDirty = localSettings.some(setting => setting.dirty); + return ( { updateRoute('integrations'); }} - dirty={localSettings.some(setting => setting.dirty)} - okColor='black' - okLabel='Save & close' + cancelLabel='Close' + dirty={isDirty} + okColor={okProps.color} + okLabel={okProps.label || 'Save'} testId='slack-modal' title='' onOk={async () => { toast.remove(); - if (await handleSave()) { - modal.remove(); - updateRoute('integrations'); - } + await handleSave(); }} > { const {updateRoute} = useRouting(); - const modal = NiceModal.useModal(); const {settings} = useGlobalData(); const [unsplashEnabled] = getSettingValues(settings, ['unsplash']); const {mutateAsync: editSettings} = useEditSettings(); const handleError = useHandleError(); + const [okLabel, setOkLabel] = useState('Save'); + const [enabled, setEnabled] = useState(!!unsplashEnabled); - const handleToggleChange = async (e: React.ChangeEvent) => { + useEffect(() => { + setEnabled(unsplashEnabled || false); + }, [unsplashEnabled]); + + const handleToggleChange = async () => { const updates: Setting[] = [ - {key: 'unsplash', value: (e.target.checked)} + {key: 'unsplash', value: (enabled)} ]; try { - await editSettings(updates); + setOkLabel('Saving...'); + await Promise.all([ + editSettings(updates), + new Promise((resolve) => { + setTimeout(resolve, 1000); + }) + ]); + setOkLabel('Saved'); } catch (error) { handleError(error); + } finally { + setTimeout(() => setOkLabel('Save'), 1000); } }; + const isDirty = !(enabled === unsplashEnabled); + return ( { updateRoute('integrations'); }} - okColor='black' - okLabel='Save & close' + cancelLabel='Close' + dirty={isDirty} + okColor={okLabel === 'Saved' ? 'green' : 'black'} + okLabel={okLabel} testId='unsplash-modal' title='' - onOk={() => { - modal.remove(); - updateRoute('integrations'); - }} + onOk={handleToggleChange} > {
Enable Unsplash image integration for your posts} label='Enable Unsplash' - onChange={handleToggleChange} + onChange={(e) => { + setEnabled(e.target.checked); + }} />
diff --git a/apps/admin-x-settings/test/acceptance/advanced/integrations/amp.test.ts b/apps/admin-x-settings/test/acceptance/advanced/integrations/amp.test.ts index d08eb1a7d2..354baa046d 100644 --- a/apps/admin-x-settings/test/acceptance/advanced/integrations/amp.test.ts +++ b/apps/admin-x-settings/test/acceptance/advanced/integrations/amp.test.ts @@ -51,7 +51,7 @@ test.describe('AMP integration', async () => { const ampToggle = ampModal.getByRole('switch'); await ampToggle.click(); - await ampModal.getByRole('button', {name: 'Cancel'}).click(); + await ampModal.getByRole('button', {name: 'Close'}).click(); await expect(page.getByTestId('confirmation-modal')).toHaveText(/leave/i); diff --git a/apps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.ts b/apps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.ts index aa6adc5367..42b71fe775 100644 --- a/apps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.ts +++ b/apps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.ts @@ -140,7 +140,7 @@ test.describe('Custom integrations', async () => { await modal.getByLabel('Description').fill('Test description'); - await modal.getByRole('button', {name: 'Cancel'}).click(); + await modal.getByRole('button', {name: 'Close'}).click(); await expect(page.getByTestId('confirmation-modal')).toHaveText(/leave/i); @@ -190,7 +190,8 @@ test.describe('Custom integrations', async () => { // Edit integration await modal.getByLabel('Description').fill('Test description'); - await modal.getByRole('button', {name: 'Save & close'}).click(); + await modal.getByRole('button', {name: 'Save'}).click(); + await modal.getByRole('button', {name: 'Close'}).click(); await expect(integrationsSection).toHaveText(/Test description/); diff --git a/apps/admin-x-settings/test/acceptance/advanced/integrations/firstPromoter.test.ts b/apps/admin-x-settings/test/acceptance/advanced/integrations/firstPromoter.test.ts index ea216d6ac9..4f6b9c45cd 100644 --- a/apps/admin-x-settings/test/acceptance/advanced/integrations/firstPromoter.test.ts +++ b/apps/admin-x-settings/test/acceptance/advanced/integrations/firstPromoter.test.ts @@ -51,7 +51,7 @@ test.describe('First Promoter integration', async () => { const fpToggle = fpModal.getByRole('switch'); await fpToggle.click(); - await fpModal.getByRole('button', {name: 'Cancel'}).click(); + await fpModal.getByRole('button', {name: 'Close'}).click(); await expect(page.getByTestId('confirmation-modal')).toHaveText(/leave/i); diff --git a/apps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.ts b/apps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.ts index 1e98e3fae2..470cb8b671 100644 --- a/apps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.ts +++ b/apps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.ts @@ -23,14 +23,15 @@ test.describe('Slack integration', async () => { // Failing validation await slackModal.getByLabel('Webhook URL').fill('badurl'); - await slackModal.getByRole('button', {name: 'Save & close'}).click(); + await slackModal.getByRole('button', {name: 'Save'}).click(); await expect(slackModal).toContainText('The URL must be in a format like https://hooks.slack.com/services/'); // Successful save await slackModal.getByLabel('Webhook URL').fill('https://hooks.slack.com/services/123456789/123456789/123456789'); await slackModal.getByLabel('Username').fill('My site'); - await slackModal.getByRole('button', {name: 'Save & close'}).click(); + await slackModal.getByRole('button', {name: 'Save'}).click(); + await slackModal.getByRole('button', {name: 'Close'}).click(); await expect(slackModal).toHaveCount(0); @@ -59,7 +60,7 @@ test.describe('Slack integration', async () => { await slackModal.getByLabel('Webhook URL').fill('https://hooks.slack.com/services/123456789/123456789/123456789'); - await slackModal.getByRole('button', {name: 'Cancel'}).click(); + await slackModal.getByRole('button', {name: 'Close'}).click(); await expect(page.getByTestId('confirmation-modal')).toHaveText(/leave/i); @@ -90,7 +91,7 @@ test.describe('Slack integration', async () => { // Doesn't send the request when validation fails await slackModal.getByLabel('Webhook URL').fill('badurl'); - await slackModal.getByRole('button', {name: 'Save & close'}).click(); + await slackModal.getByRole('button', {name: 'Save'}).click(); await expect(slackModal).toContainText('The URL must be in a format like https://hooks.slack.com/services/'); expect(lastApiRequests.testSlack).toBeUndefined(); diff --git a/apps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.ts b/apps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.ts index 21bf9ef5ba..786ba01b39 100644 --- a/apps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.ts +++ b/apps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.ts @@ -21,6 +21,8 @@ test.describe('Unsplash integration', async () => { const unsplashToggle = unsplashModal.getByRole('switch'); await unsplashToggle.click(); + await unsplashModal.getByRole('button', {name: 'Save'}).click(); + expect(lastApiRequests.editSettings?.body).toEqual({ settings: [ {key: 'unsplash', value: false}