Skip to content

Commit

Permalink
🎨 Improved Access card layout in Settings (#21920)
Browse files Browse the repository at this point in the history
As part of our efforts to add more personality to Settings, we've
identified areas where we can improve the UX.

This change makes it easier to manipulate Access settings by removing a
step from the flow.

fixes
https://linear.app/ghost/issue/DES-484/improve-access-card-layout-in-settings

**Before**
<img width="1718" alt="access-card-before"
src="https://github.com/user-attachments/assets/8c0f8f14-31b9-4712-93d2-97eb0f05c965"
/>

**After**
<img width="1718" alt="access-card-after"
src="https://github.com/user-attachments/assets/5831a7cc-fbad-4d4b-bccc-6e86be6a8c65"
/>
  • Loading branch information
dvdwinden authored Dec 19, 2024
1 parent d2b7872 commit ab2c7f1
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ export interface SettingGroupContentProps {
}

const SettingGroupContent: React.FC<SettingGroupContentProps> = ({columns, values, children, className}) => {
let styles = 'flex flex-col gap-x-5 gap-y-7';
let styles = 'flex flex-col gap-x-5';
if (columns === 2) {
styles = 'grid grid-cols-1 md:grid-cols-2 gap-x-8 gap-y-6';
}

styles += ` ${className}`;
styles += className ? ` ${className}` : ' gap-y-7';

return (
<div className={styles}>
Expand Down
156 changes: 89 additions & 67 deletions apps/admin-x-settings/src/components/settings/membership/Access.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import React from 'react';
import TopLevelGroup from '../../TopLevelGroup';
import useSettingGroup from '../../../hooks/useSettingGroup';
import {GroupBase, MultiValue} from 'react-select';
import {MultiSelect, MultiSelectOption, Select, SettingGroupContent, withErrorBoundary} from '@tryghost/admin-x-design-system';
import {getOptionLabel} from '../../../utils/helpers';
import {MultiSelect, MultiSelectOption, Select, Separator, SettingGroupContent, withErrorBoundary} from '@tryghost/admin-x-design-system';
// import {getOptionLabel} from '../../../utils/helpers';
import {getSettingValues} from '@tryghost/admin-x-framework/api/settings';
import {useBrowseTiers} from '@tryghost/admin-x-framework/api/tiers';

Expand Down Expand Up @@ -81,9 +81,9 @@ const Access: React.FC<{ keywords: string[] }> = ({keywords}) => {
'members_signup_access', 'default_content_visibility', 'default_content_visibility_tiers', 'comments_enabled'
]) as string[];

const membersSignupAccessLabel = getOptionLabel(MEMBERS_SIGNUP_ACCESS_OPTIONS, membersSignupAccess);
const defaultContentVisibilityLabel = getOptionLabel(DEFAULT_CONTENT_VISIBILITY_OPTIONS, defaultContentVisibility);
const commentsEnabledLabel = getOptionLabel(COMMENTS_ENABLED_OPTIONS, commentsEnabled);
// const membersSignupAccessLabel = getOptionLabel(MEMBERS_SIGNUP_ACCESS_OPTIONS, membersSignupAccess);
// const defaultContentVisibilityLabel = getOptionLabel(DEFAULT_CONTENT_VISIBILITY_OPTIONS, defaultContentVisibility);
// const commentsEnabledLabel = getOptionLabel(COMMENTS_ENABLED_OPTIONS, commentsEnabled);

const {data: {tiers} = {}} = useBrowseTiers();

Expand All @@ -106,71 +106,92 @@ const Access: React.FC<{ keywords: string[] }> = ({keywords}) => {
updateSetting('default_content_visibility_tiers', JSON.stringify(selectedTiers));
};

const values = (
<SettingGroupContent
values={[
{
heading: 'Subscription access',
key: 'subscription-access',
value: membersSignupAccessLabel
},
{
heading: 'Default post access',
key: 'default-post-access',
value: defaultContentVisibilityLabel
},
{
heading: 'Commenting',
key: 'commenting',
value: commentsEnabledLabel
}
]}
/>
);
// const values = (
// <SettingGroupContent
// values={[
// {
// heading: 'Subscription access',
// key: 'subscription-access',
// value: membersSignupAccessLabel
// },
// {
// heading: 'Default post access',
// key: 'default-post-access',
// value: defaultContentVisibilityLabel
// },
// {
// heading: 'Commenting',
// key: 'commenting',
// value: commentsEnabledLabel
// }
// ]}
// />
// );

const form = (
<SettingGroupContent columns={1}>
<Select
hint='Who should be able to subscribe to your site?'
options={MEMBERS_SIGNUP_ACCESS_OPTIONS}
selectedOption={MEMBERS_SIGNUP_ACCESS_OPTIONS.find(option => option.value === membersSignupAccess)}
testId='subscription-access-select'
title="Subscription access"
onSelect={(option) => {
updateSetting('members_signup_access', option?.value || null);
}}
/>
<Select
hint='When a new post is created, who should have access?'
options={DEFAULT_CONTENT_VISIBILITY_OPTIONS}
selectedOption={DEFAULT_CONTENT_VISIBILITY_OPTIONS.find(option => option.value === defaultContentVisibility)}
testId='default-post-access-select'
title="Default post access"
onSelect={(option) => {
updateSetting('default_content_visibility', option?.value || null);
}}
/>
<SettingGroupContent className='gap-y-4' columns={1}>
<div className="flex flex-col content-center items-center gap-4 md:flex-row">
<div className="w-full min-w-[160px] max-w-none md:w-2/3 md:max-w-[320px]">Who should be able to subscribe to your site?</div>
<div className="w-full md:flex-1">
<Select
options={MEMBERS_SIGNUP_ACCESS_OPTIONS}
selectedOption={MEMBERS_SIGNUP_ACCESS_OPTIONS.find(option => option.value === membersSignupAccess)}
testId='subscription-access-select'
onSelect={(option) => {
updateSetting('members_signup_access', option?.value || null);
handleEditingChange(true);
}}
/>
</div>
</div>
<Separator />
<div className="flex flex-col content-center items-center gap-4 md:flex-row">
<div className="w-full min-w-[160px] max-w-none md:w-2/3 md:max-w-[320px]">Who should have access to new posts?</div>
<div className="w-full md:flex-1">
<Select
options={DEFAULT_CONTENT_VISIBILITY_OPTIONS}
selectedOption={DEFAULT_CONTENT_VISIBILITY_OPTIONS.find(option => option.value === defaultContentVisibility)}
testId='default-post-access-select'
onSelect={(option) => {
updateSetting('default_content_visibility', option?.value || null);
handleEditingChange(true);
}}
/>
</div>
</div>
{defaultContentVisibility === 'tiers' && (
<MultiSelect
color='black'
options={tierOptionGroups.filter(group => group.options.length > 0)}
testId='tiers-select'
title='Select tiers'
values={selectedTierOptions}
clearBg
onChange={setSelectedTiers}
/>
<div className="flex flex-col content-center items-center gap-4 md:flex-row">
<div className="w-full min-w-[160px] max-w-none md:w-2/3 md:max-w-[320px]">Select specific tiers</div>
<div className="w-full md:flex-1">
<MultiSelect
color='black'
options={tierOptionGroups.filter(group => group.options.length > 0)}
testId='tiers-select'
values={selectedTierOptions}
onChange={(selectedOptions) => {
setSelectedTiers(selectedOptions);
handleEditingChange(true);
}}
/>
</div>
</div>
)}
<Select
hint='Who can comment on posts?'
options={COMMENTS_ENABLED_OPTIONS}
selectedOption={COMMENTS_ENABLED_OPTIONS.find(option => option.value === commentsEnabled)}
testId='commenting-select'
title="Commenting"
onSelect={(option) => {
updateSetting('comments_enabled', option?.value || null);
}}
/>
<Separator />
<div className="flex flex-col content-center items-center gap-4 md:flex-row">
<div className="w-full min-w-[160px] max-w-none md:w-2/3 md:max-w-[320px]">Who can comment on posts?</div>
<div className="w-full md:flex-1">
<Select
options={COMMENTS_ENABLED_OPTIONS}
selectedOption={COMMENTS_ENABLED_OPTIONS.find(option => option.value === commentsEnabled)}
testId='commenting-select'
title=""
onSelect={(option) => {
updateSetting('comments_enabled', option?.value || null);
handleEditingChange(true);
}}
/>
</div>
</div>
</SettingGroupContent>
);

Expand All @@ -183,11 +204,12 @@ const Access: React.FC<{ keywords: string[] }> = ({keywords}) => {
saveState={saveState}
testId='access'
title='Access'
hideEditButton
onCancel={handleCancel}
onEditingChange={handleEditingChange}
onSave={handleSave}
>
{isEditing ? form : values}
{form}
</TopLevelGroup>
);
};
Expand Down
12 changes: 2 additions & 10 deletions apps/admin-x-settings/test/acceptance/membership/access.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,12 @@ test.describe('Access settings', async () => {
await expect(section.getByText('Public')).toHaveCount(1);
await expect(section.getByText('Nobody')).toHaveCount(1);

await section.getByRole('button', {name: 'Edit'}).click();

await chooseOptionInSelect(section.getByTestId('subscription-access-select'), 'Only people I invite');
await chooseOptionInSelect(section.getByTestId('default-post-access-select'), /^Members only$/);
await chooseOptionInSelect(section.getByTestId('commenting-select'), 'All members');

await section.getByRole('button', {name: 'Save'}).click();

await expect(section.getByTestId('subscription-access-select')).toHaveCount(0);

await expect(section.getByText('Only people I invite')).toHaveCount(1);
await expect(section.getByText('Members only')).toHaveCount(1);
await expect(section.getByText('All members')).toHaveCount(1);
Expand All @@ -56,8 +52,6 @@ test.describe('Access settings', async () => {

const section = page.getByTestId('access');

await section.getByRole('button', {name: 'Edit'}).click();

await chooseOptionInSelect(section.getByTestId('subscription-access-select'), 'Nobody');

await section.getByRole('button', {name: 'Save'}).click();
Expand All @@ -68,7 +62,7 @@ test.describe('Access settings', async () => {
]
});

await expect(section.getByTestId('subscription-access-select')).toHaveCount(0);
await expect(section.getByTestId('subscription-access-select')).toContainText('Nobody');

await expect(page.getByTestId('portal').getByRole('button', {name: 'Customize'})).toBeDisabled();
await expect(page.getByTestId('enable-newsletters')).toContainText('only existing members will receive newsletters');
Expand All @@ -88,8 +82,6 @@ test.describe('Access settings', async () => {

const section = page.getByTestId('access');

await section.getByRole('button', {name: 'Edit'}).click();

await chooseOptionInSelect(section.getByTestId('default-post-access-select'), 'Specific tiers');
await section.getByTestId('tiers-select').click();

Expand All @@ -98,7 +90,7 @@ test.describe('Access settings', async () => {

await section.getByRole('button', {name: 'Save'}).click();

await expect(section.getByText('Specific tiers')).toHaveCount(1);
await expect(section.getByTestId('default-post-access-select')).toContainText('Specific tiers');

expect(lastApiRequests.editSettings?.body).toEqual({
settings: [
Expand Down
10 changes: 7 additions & 3 deletions ghost/core/test/e2e-browser/admin/site-settings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@ const changeSubscriptionAccess = async (page, access) => {
await page.locator('[data-test-nav="settings"]').click();

const section = page.getByTestId('access');
await section.getByRole('button', {name: 'Edit'}).click();


const select = section.getByTestId('subscription-access-select');
await select.click();
await page.locator(`[data-testid="select-option"][data-value="${access}"]`).click();

// Save settings
await section.getByRole('button', {name: 'Save'}).click();
await expect(select).not.toBeVisible();

await expect(select).toContainText(
access === 'all' ? 'Anyone can sign up' :
access === 'invite' ? 'Only people I invite' :
'Nobody'
);
};

const checkPortalScriptLoaded = async (page, loaded = true) => {
Expand Down

0 comments on commit ab2c7f1

Please sign in to comment.