TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-09-11 12:55:37 +03:00
Code Issues Projects Releases Wiki Activity

Unify "Save" and "Close" buttons in Settings (#20430)

Browse Source 
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.
This commit is contained in:
Peter Zimon 2024-07-01 13:35:38 +02:00 committed by GitHub
parent 95a4895e8f
commit 6378d7d66f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 28 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
apps/admin-x-settings/src/components/settings/general/UserDetailModal.tsx
View File

@ -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'

8
apps/admin-x-settings/src/components/settings/growth/recommendations/EditRecommendationModal.tsx
View File

@ -27,10 +27,6 @@ const EditRecommendationModal: React.FC<RoutingModalProps & EditRecommendationMo
onSave: async (state) => {
await editRecommendation(state);
},
onSavedStateReset: () => {
modal.remove();
updateRoute('recommendations');
},
onSaveError: handleError,
onValidate: (state) => {
const newErrors = validateDescriptionForm(state);
@ -76,10 +72,10 @@ const EditRecommendationModal: React.FC<RoutingModalProps & EditRecommendationMo
animate={animate ?? true}
backDropClick={false}
buttonsDisabled={okProps.disabled}
cancelLabel={'Cancel'}
cancelLabel={'Close'}
leftButtonProps={leftButtonProps}
okColor={okProps.color}
okLabel={okProps.label || 'Save & close'}
okLabel={okProps.label || 'Save'}
size='sm'
testId='edit-recommendation-modal'
title={'Edit recommendation'}

10
apps/admin-x-settings/src/components/settings/membership/tiers/TierDetailModal.tsx
View File

@ -1,4 +1,4 @@
import NiceModal, {useModal} from '@ebay/nice-modal-react';
import NiceModal from '@ebay/nice-modal-react';
import React, {useEffect, useRef} from 'react';
import TierDetailPreview from './TierDetailPreview';
import useFeatureFlag from '../../../../hooks/useFeatureFlag';
@ -17,7 +17,6 @@ export type TierFormState = Partial<Omit<Tier, 'trial_days'>> & {
const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => {
const isFreeTier = tier?.type === 'free';
const modal = useModal();
const {updateRoute} = useRouting();
const {mutateAsync: updateTier} = useEditTier();
const {mutateAsync: createTier} = useAddTier();
@ -97,10 +96,6 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => {
}
}
},
onSavedStateReset: () => {
modal.remove();
updateRoute('tiers');
},
onSaveError: handleError
});
@ -185,10 +180,11 @@ const TierDetailModalContent: React.FC<{tier?: Tier}> = ({tier}) => {
updateRoute('tiers');
}}
buttonsDisabled={okProps.disabled}
cancelLabel='Close'
dirty={saveState === 'unsaved'}
leftButtonProps={leftButtonProps}
okColor={okProps.color}
okLabel={okProps.label || 'Save & close'}
okLabel={okProps.label || 'Save'}
size='lg'
testId='tier-detail-modal'
title={(tier ? (tier.active ? 'Edit tier' : 'Edit archived tier') : 'New tier')}

17
apps/admin-x-settings/test/acceptance/general/users/profile.test.ts
View File

@ -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');
@ -166,9 +166,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();
@ -314,7 +315,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);
@ -374,7 +375,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

5
apps/admin-x-settings/test/acceptance/general/users/roles.test.ts
View File

@ -69,10 +69,12 @@ test.describe('User roles', async () => {
await modal.locator('input[value=editor]').check();
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(activeTab).toHaveText(/No authors found./);
await section.getByRole('tab', {name: 'Editors'}).click();
@ -146,6 +148,7 @@ test.describe('User roles', async () => {
await modal.getByLabel('Full name').fill('New name');
await modal.getByRole('button', {name: 'Save'}).click();
await modal.getByRole('button', {name: 'Close'}).click();
await expect(modal).toBeHidden();
});

13
apps/admin-x-settings/test/acceptance/membership/tiers.test.ts
View File

@ -18,7 +18,7 @@ test.describe('Tier settings', async () => {
const modal = page.getByTestId('tier-detail-modal');
await modal.getByRole('button', {name: 'Save & close'}).click();
await modal.getByRole('button', {name: 'Save'}).click();
await expect(modal).toHaveText(/Enter a name for the tier/);
await expect(modal).toHaveText(/Amount must be at least \$1/);
@ -51,7 +51,8 @@ test.describe('Tier settings', async () => {
browseTiers: {method: 'GET', path: '/tiers/', response: {tiers: [...responseFixtures.tiers.tiers, newTier]}}
}});
await modal.getByRole('button', {name: 'Save & close'}).click();
await modal.getByRole('button', {name: 'Save'}).click();
await modal.getByRole('button', {name: 'Close'}).click();
// await expect(section.getByTestId('tier-card').filter({hasText: /Plus/})).toHaveText(/Plus tier/);
// await expect(section.getByTestId('tier-card').filter({hasText: /Plus/})).toHaveText(/\$8\/month/);
@ -103,7 +104,7 @@ test.describe('Tier settings', async () => {
// Failing validations
await modal.getByLabel('Name').fill('');
await modal.getByRole('button', {name: 'Save & close'}).click();
await modal.getByRole('button', {name: 'Save'}).click();
await expect(modal).toHaveText(/Enter a name for the tier/);
@ -132,7 +133,8 @@ test.describe('Tier settings', async () => {
// Save changes
await modal.getByRole('button', {name: 'Save & close'}).click();
await modal.getByRole('button', {name: 'Save'}).click();
await modal.getByRole('button', {name: 'Close'}).click();
await expect(section.getByTestId('tier-card').filter({hasText: /Supporter/})).toHaveText(/Supporter updated/);
await expect(section.getByTestId('tier-card').filter({hasText: /Supporter/})).toHaveText(/Supporter description/);
@ -185,7 +187,8 @@ test.describe('Tier settings', async () => {
await modal.getByRole('button', {name: 'Add'}).click();
await modal.getByLabel('New benefit').fill('Second benefit');
await modal.getByRole('button', {name: 'Save & close'}).click();
await modal.getByRole('button', {name: 'Save'}).click();
await modal.getByRole('button', {name: 'Close'}).click();
await expect(section.getByTestId('tier-card').filter({hasText: /Free/})).toHaveText(/Free tier description/);