Unify "Save" and "Close" buttons in Settings

DES-27 There are two patterns used in settings modals for action buttons: 1. [Cancel] and [Save & close] (sometimes it's [Cancel] and [OK], inconsistently) — example: Staff details, Tier details, Navigation, Recommendation 2. [Close] and [Save] — example: Design settings, Portal, Newsletter details etc. This is confusing and leaves people confused and uncertain about what's going to happen in one or the other case.
TryGhost · Jun 20, 2024 · 763e62e · 763e62e
1 parent 1c972c7
commit 763e62e
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 13 deletions.
diff --git a/apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx b/apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx
@@ -113,10 +113,6 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
         onSave: async (values) => {
             await updateUser?.(values);
         },
-        onSavedStateReset: () => {
-            mainModal.remove();
-            navigateOnClose();
-        },
         onSaveError: handleError
     });
     const setUserData = (newData: User) => updateForm(() => newData);
@@ -353,9 +349,10 @@ const UserDetailModalContent: React.FC<{user: User}> = ({user}) => {
             animate={canAccessSettings(currentUser)}
             backDrop={canAccessSettings(currentUser)}
             buttonsDisabled={okProps.disabled}
+            cancelLabel='Close'
             dirty={saveState === 'unsaved'}
             okColor={okProps.color}
-            okLabel={okProps.label || 'Save & close'}
+            okLabel={okProps.label || 'Save'}
             size={canAccessSettings(currentUser) ? 'lg' : 'bleed'}
             stickyFooter={true}
             testId='user-detail-modal'

diff --git a/apps/admin-x-settings/test/acceptance/general/users/profile.test.ts b/apps/admin-x-settings/test/acceptance/general/users/profile.test.ts
@@ -35,23 +35,23 @@ test.describe('User profile', async () => {
         // Validation failures
 
         await modal.getByLabel('Full name').fill('');
-        await modal.getByRole('button', {name: 'Save & close'}).click();
+        await modal.getByRole('button', {name: 'Save'}).click();
         await expect(modal).toContainText('Name is required');
 
         await modal.getByLabel('Email').fill('test');
-        await modal.getByRole('button', {name: 'Save & close'}).click();
+        await modal.getByRole('button', {name: 'Save'}).click();
         await expect(modal).toContainText('Enter a valid email address');
 
         await modal.getByLabel('Location').fill(new Array(195).join('a'));
-        await modal.getByRole('button', {name: 'Save & close'}).click();
+        await modal.getByRole('button', {name: 'Save'}).click();
         await expect(modal).toContainText('Location is too long');
 
         await modal.getByLabel('Bio').fill(new Array(210).join('a'));
-        await modal.getByRole('button', {name: 'Save & close'}).click();
+        await modal.getByRole('button', {name: 'Save'}).click();
         await expect(modal).toContainText('Bio is too long');
 
         await modal.getByLabel('Website').fill('not-a-website');
-        await modal.getByRole('button', {name: 'Save & close'}).click();
+        await modal.getByRole('button', {name: 'Save'}).click();
         await expect(modal).toContainText('Enter a valid URL');
 
         const facebookInput = modal.getByLabel('Facebook profile');
@@ -165,9 +165,10 @@ test.describe('User profile', async () => {
         await modal.getByLabel(/Paid member cancellations/).check();
         await modal.getByLabel(/Milestones/).uncheck();
 
-        await modal.getByRole('button', {name: 'Save & close'}).click();
+        await modal.getByRole('button', {name: 'Save'}).click();
 
         await expect(modal.getByRole('button', {name: 'Saved'})).toBeVisible();
+        await modal.getByRole('button', {name: 'Close'}).click();
 
         await expect(listItem.getByText('New Admin')).toBeVisible();
         await expect(listItem.getByText('newadmin@test.com')).toBeVisible();
@@ -313,7 +314,7 @@ test.describe('User profile', async () => {
 
         await modal.getByLabel('Full name').fill('Updated');
 
-        await modal.getByRole('button', {name: 'Cancel'}).click();
+        await modal.getByRole('button', {name: 'Close'}).click();
 
         await expect(page.getByTestId('confirmation-modal')).toHaveText(/leave/i);
 
@@ -373,7 +374,7 @@ test.describe('User profile', async () => {
         await listItem.getByRole('button', {name: 'Edit'}).click();
 
         await expect(modal.getByTestId('api-keys')).toBeHidden();
-        await modal.getByRole('button', {name: 'Cancel'}).click();
+        await modal.getByRole('button', {name: 'Close'}).click();
 
         // Can see and regenerate your own staff token