[Website] Restore the single-click "Edit Settings" flow (#1854)

## Motivation for the change, related issues Restores the easily accessible "Edit settings" button Playground had before merging #1731, and refocuses the user experience on a single, temporary Playground. Multiple Playgrounds are still possible, but now they're less emphasized. As we've learned from @annezazu and other users, the recent [Multiple Playgrounds UI update](#1731) made rapid fire iterations in Playground more difficult. Before #1731, we've had an easily accessible button to update WP and PHP versions. After #1731, that feature required multiple clicks and finding the right button. This PR introduces the following changes: * Add an easily-accessible "Site settings" button for quick PHP/WP version updates * The URL reflects the Query API values used to create the temporary Playground. * Limit the number of temporary Playground sites to exactly one – just like before #1731. The temporary Playground is always there and cannot be deleted. * The only way to create a stored Playground is by saving the temporary Playground. Once you do that, you get a fresh temporary Playground. Kudos to @jarekmorawski for thinking through and designing multiple variations of the user flows ❤️ ## Technical details The implementation is straightforward and focused on rearranging React components and CSS. There's one gotcha in the process of saving temporary site settings. The settings form submission calls `window.history.pushState()` and the `EnsurePlaygroundIsSelected` component watches for the URL changes. However, the user may click the "Update Settings & Reset Playground" button even without changing any form value. Normally, this would mean we can't detect a change and reset the Playground. This PR, thus, adds the `?random=<random string>` parameter to the query string to allow Playground notice the change and recreate the temporary site. ## Visuals ![CleanShot 2024-10-06 at 23 19 12@2x](https://github.com/user-attachments/assets/11e69587-a6ca-4f73-8d51-f15997950e71) ![CleanShot 2024-10-07 at 01 35 12@2x](https://github.com/user-attachments/assets/0e13f94a-adef-4f5a-8fc3-f3f4b9a577c6) Here's the video walkthrough – note I've recorded it before this PR was fully ready for a review: https://github.com/user-attachments/assets/b2f04fa6-d7d5-43ad-93e2-975a2a9cea21 ## Follow up work There are more design elements to consider, e.g. Snackbar notices. @jarekmorawski already designed some improvements and is working more. I would still like to get this PR in and continue iterating based on the feedback we get. ## UI updates checklist - [x] Tested mobile interactions - [x] Resolved accessibility issues reported by Axe Devtools ## Testing plan CI aside, interact with the design proposed in this PR and confirm it feels right.
WordPress · Oct 7, 2024 · 707a733 · 707a733
1 parent cf2f48d
commit 707a733
Show file tree

Hide file tree

Showing 31 changed files with 1,338 additions and 1,542 deletions.
diff --git a/packages/playground/website/index.html b/packages/playground/website/index.html
@@ -43,7 +43,7 @@
 		</script>
 	</head>
 	<body>
-		<main id="root">
+		<main id="root" aria-label="WordPress Playground">
 			<script>
 				const query = new URLSearchParams(window.location.search);
 				const shouldLazyLoadPlayground =

diff --git a/packages/playground/website/playwright/e2e/website-ui.spec.ts b/packages/playground/website/playwright/e2e/website-ui.spec.ts
@@ -13,7 +13,7 @@ test('should reflect the URL update from the navigation bar in the WordPress sit
 	website,
 }) => {
 	await website.goto('./?url=/wp-admin/');
-	await website.ensureSiteViewIsExpanded();
+	await website.ensureSiteManagerIsClosed();
 	await expect(website.page.locator('input[value="/wp-admin/"]')).toHaveValue(
 		'/wp-admin/'
 	);
@@ -23,74 +23,98 @@ test('should correctly load /wp-admin without the trailing slash', async ({
 	website,
 }) => {
 	await website.goto('./?url=/wp-admin');
-	await website.ensureSiteViewIsExpanded();
+	await website.ensureSiteManagerIsClosed();
 	await expect(website.page.locator('input[value="/wp-admin/"]')).toHaveValue(
 		'/wp-admin/'
 	);
 });
 
-test('should switch between sites', async ({ website }) => {
+test('should switch between sites', async ({ website, browserName }) => {
+	test.skip(
+		browserName === 'webkit',
+		`This test relies on OPFS which isn't available in Playwright's flavor of Safari.`
+	);
+
 	await website.goto('./');
 
 	await website.ensureSiteManagerIsOpen();
-	await website.openNewSiteModal();
-
-	const newSiteName = await website.page
-		.locator('input[placeholder="Playground name"]')
-		.inputValue();
 
-	await website.clickCreateInNewSiteModal();
+	await expect(website.page.getByText('Save')).toBeEnabled();
+	await website.page.getByText('Save').click();
+	// We shouldn't need to explicitly call .waitFor(), but the test fails without it.
+	// Playwright logs that something "intercepts pointer events", that's probably related.
+	await website.page.getByText('Save in this browser').waitFor();
+	await website.page.getByText('Save in this browser').click({ force: true });
+	await expect(
+		website.page.locator('[aria-current="page"]')
+	).not.toContainText('Temporary Playground', {
+		// Saving the site takes a while on CI
+		timeout: 90000,
+	});
+	await expect(website.page.getByLabel('Playground title')).not.toContainText(
+		'Temporary Playground'
+	);
 
-	await expect(await website.getSiteTitle()).toMatch(newSiteName);
+	await website.page
+		.locator('button')
+		.filter({ hasText: 'Temporary Playground' })
+		.click();
 
-	const sidebarItem = website.page.locator(
-		'.components-button[class*="_sidebar-item"]:not([class*="_sidebar-item--selected_"])'
+	await expect(website.page.locator('[aria-current="page"]')).toContainText(
+		'Temporary Playground'
+	);
+	await expect(website.page.getByLabel('Playground title')).toContainText(
+		'Temporary Playground'
 	);
-	const siteName = await sidebarItem
-		.locator('.components-flex-item[class*="_sidebar-item--site-name"]')
-		.innerText();
-	await sidebarItem.click();
-
-	await expect(siteName).toMatch(await website.getSiteTitle());
 });
 
 SupportedPHPVersions.forEach(async (version) => {
+	/**
+	 * WordPress 6.6 dropped support for PHP 7.0 and 7.1 and won't load on these versions.
+	 * Therefore, we're skipping the test for these versions.
+	 * @see https://make.wordpress.org/core/2024/04/08/dropping-support-for-php-7-1/
+	 */
+	if (['7.0', '7.1'].includes(version)) {
+		return;
+	}
+
 	test(`should switch PHP version to ${version}`, async ({ website }) => {
-		/**
-		 * WordPress 6.6 dropped support for PHP 7.0 and 7.1 so we need to skip these versions.
-		 * @see https://make.wordpress.org/core/2024/04/08/dropping-support-for-php-7-1/
-		 */
-		if (['7.0', '7.1'].includes(version)) {
-			return;
-		}
 		await website.goto(`./`);
-		await website.openForkPlaygroundSettings();
-		await website.selectPHPVersion(version);
-		await website.clickSaveInForkPlaygroundSettings();
-
-		await expect(website.getSiteInfoRowLocator('PHP version')).toHaveText(
-			`${version} (with extensions)`
+		await website.ensureSiteManagerIsOpen();
+		await website.page.getByLabel('PHP version').selectOption(version);
+		await website.page
+			.getByText('Apply Settings & Reset Playground')
+			.click();
+		await website.ensureSiteManagerIsClosed();
+		await website.ensureSiteManagerIsOpen();
+
+		await expect(website.page.getByLabel('PHP version')).toHaveValue(
+			version
 		);
+		await expect(
+			website.page.getByLabel('Load PHP extensions')
+		).toBeChecked();
 	});
 
 	test(`should not load additional PHP ${version} extensions when not requested`, async ({
 		website,
 	}) => {
 		await website.goto('./');
-		await website.openForkPlaygroundSettings();
-		await website.selectPHPVersion(version);
-
-		// Uncheck the "with extensions" checkbox
-		const phpExtensionCheckbox = website.page.locator(
-			'.components-checkbox-control__input[name="withExtensions"]'
-		);
-		await phpExtensionCheckbox.uncheck();
-
-		await website.clickSaveInForkPlaygroundSettings();
-
-		await expect(website.getSiteInfoRowLocator('PHP version')).toHaveText(
+		await website.ensureSiteManagerIsOpen();
+		await website.page.getByLabel('PHP version').selectOption(version);
+		await website.page.getByLabel('Load PHP extensions').uncheck();
+		await website.page
+			.getByText('Apply Settings & Reset Playground')
+			.click();
+		await website.ensureSiteManagerIsClosed();
+		await website.ensureSiteManagerIsOpen();
+
+		await expect(website.page.getByLabel('PHP version')).toHaveValue(
 			version
 		);
+		await expect(
+			website.page.getByLabel('Load PHP extensions')
+		).not.toBeChecked();
 	});
 });
 
@@ -102,13 +126,19 @@ Object.keys(MinifiedWordPressVersions)
 			website,
 		}) => {
 			await website.goto('./');
-			await website.openForkPlaygroundSettings();
-			await website.selectWordPressVersion(version);
-			await website.clickSaveInForkPlaygroundSettings();
+			await website.ensureSiteManagerIsOpen();
+			await website.page
+				.getByLabel('WordPress version')
+				.selectOption(version);
+			await website.page
+				.getByText('Apply Settings & Reset Playground')
+				.click();
+			await website.ensureSiteManagerIsClosed();
+			await website.ensureSiteManagerIsOpen();
 
 			await expect(
-				website.getSiteInfoRowLocator('WordPress version')
-			).toHaveText(version);
+				website.page.getByLabel('WordPress version')
+			).toHaveValue(version);
 		});
 	});
 
@@ -117,43 +147,39 @@ test('should display networking as inactive by default', async ({
 }) => {
 	await website.goto('./');
 	await website.ensureSiteManagerIsOpen();
-	await expect(website.getSiteInfoRowLocator('Network access')).toContainText(
-		'No'
-	);
+	await expect(website.page.getByLabel('Network access')).not.toBeChecked();
 });
 
 test('should display networking as active when networking is enabled', async ({
 	website,
 }) => {
 	await website.goto('./?networking=yes');
 	await website.ensureSiteManagerIsOpen();
-	await expect(website.getSiteInfoRowLocator('Network access')).toContainText(
-		'Yes'
-	);
+	await expect(website.page.getByLabel('Network access')).toBeChecked();
 });
 
 test('should enable networking when requested', async ({ website }) => {
 	await website.goto('./');
 
-	await website.openForkPlaygroundSettings();
-	await website.setNetworkingEnabled(true);
-	await website.clickSaveInForkPlaygroundSettings();
+	await website.ensureSiteManagerIsOpen();
+	await website.page.getByLabel('Network access').check();
+	await website.page.getByText('Apply Settings & Reset Playground').click();
+	await website.ensureSiteManagerIsClosed();
+	await website.ensureSiteManagerIsOpen();
 
-	await expect(website.getSiteInfoRowLocator('Network access')).toContainText(
-		'Yes'
-	);
+	await expect(website.page.getByLabel('Network access')).toBeChecked();
 });
 
 test('should disable networking when requested', async ({ website }) => {
 	await website.goto('./?networking=yes');
 
-	await website.openForkPlaygroundSettings();
-	await website.setNetworkingEnabled(false);
-	await website.clickSaveInForkPlaygroundSettings();
+	await website.ensureSiteManagerIsOpen();
+	await website.page.getByLabel('Network access').uncheck();
+	await website.page.getByText('Apply Settings & Reset Playground').click();
+	await website.ensureSiteManagerIsClosed();
+	await website.ensureSiteManagerIsOpen();
 
-	await expect(website.getSiteInfoRowLocator('Network access')).toContainText(
-		'No'
-	);
+	await expect(website.page.getByLabel('Network access')).not.toBeChecked();
 });
 
 test('should display PHP output even when a fatal error is hit', async ({

diff --git a/packages/playground/website/playwright/website-page.ts b/packages/playground/website/playwright/website-page.ts
@@ -25,92 +25,25 @@ export class WebsitePage {
 	}
 
 	async ensureSiteManagerIsOpen() {
-		const siteManagerHeading = this.page.getByText('Your Playgrounds');
-		if (!(await siteManagerHeading.isVisible({ timeout: 5000 }))) {
+		const siteManagerHeading = this.page.locator('.main-sidebar');
+		if (await siteManagerHeading.isHidden({ timeout: 5000 })) {
 			await this.page.getByLabel('Open Site Manager').click();
 		}
 		await expect(siteManagerHeading).toBeVisible();
 	}
 
-	async ensureSiteViewIsExpanded() {
+	async ensureSiteManagerIsClosed() {
 		const openSiteButton = this.page.locator('div[title="Open site"]');
 		if (await openSiteButton.isVisible({ timeout: 5000 })) {
 			await openSiteButton.click();
 		}
-
-		const siteManagerHeading = this.page.getByText('Your Playgrounds');
+		const siteManagerHeading = this.page.locator('.main-sidebar');
 		await expect(siteManagerHeading).not.toBeVisible();
 	}
 
-	async openNewSiteModal() {
-		const addPlaygroundButton = this.page.locator(
-			'button.components-button',
-			{
-				hasText: 'Add Playground',
-			}
-		);
-		await addPlaygroundButton.click();
-	}
-
-	async clickCreateInNewSiteModal() {
-		const createTempPlaygroundButton = this.page.locator(
-			'button.components-button',
-			{
-				hasText: 'Create a temporary Playground',
-			}
-		);
-		await createTempPlaygroundButton.click();
-	}
-
 	async getSiteTitle(): Promise<string> {
 		return await this.page
 			.locator('h1[class*="_site-info-header-details-name"]')
 			.innerText();
 	}
-
-	async openForkPlaygroundSettings() {
-		await this.ensureSiteManagerIsOpen();
-		const editSettingsButton = this.page.locator(
-			'button.components-button',
-			{
-				hasText: 'Create a similar Playground',
-			}
-		);
-		await editSettingsButton.click({ timeout: 5000 });
-	}
-
-	async selectPHPVersion(version: string) {
-		const phpVersionSelect = this.page.locator('select[name=phpVersion]');
-		await phpVersionSelect.selectOption(version);
-	}
-
-	async clickSaveInForkPlaygroundSettings() {
-		const saveSettingsButton = this.page.locator(
-			'button.components-button.is-primary',
-			{
-				hasText: 'Create',
-			}
-		);
-		await saveSettingsButton.click();
-	}
-
-	async selectWordPressVersion(version: string) {
-		const wordpressVersionSelect = this.page.locator(
-			'select[name=wpVersion]'
-		);
-		await wordpressVersionSelect.selectOption(version);
-	}
-
-	getSiteInfoRowLocator(key: string) {
-		return this.page.getByLabel(key);
-	}
-
-	async setNetworkingEnabled(enabled: boolean) {
-		const checkbox = this.page.locator('input[name="withNetworking"]');
-		if (enabled) {
-			await checkbox.check();
-		} else {
-			await checkbox.uncheck();
-		}
-	}
 }
diff --git a/packages/playground/website/src/components/browser-chrome/index.tsx b/packages/playground/website/src/components/browser-chrome/index.tsx
@@ -9,6 +9,10 @@ import {
 	useActiveSite,
 } from '../../lib/state/redux/store';
 import { SyncLocalFilesButton } from '../sync-local-files-button';
+import { Dropdown, Icon } from '@wordpress/components';
+import { cog } from '@wordpress/icons';
+import Button from '../button';
+import { ActiveSiteSettingsForm } from '../site-manager/site-settings-form';
 
 interface BrowserChromeProps {
 	children?: React.ReactNode;
@@ -57,6 +61,45 @@ export default function BrowserChrome({
 					</div>
 
 					<div className={css.toolbarButtons}>
+						<Dropdown
+							className="my-container-class-name"
+							contentClassName="my-dropdown-content-classname"
+							popoverProps={{ placement: 'bottom-start' }}
+							renderToggle={({ isOpen, onToggle }) => (
+								<Button
+									variant="browser-chrome"
+									aria-label="Edit Playground settings"
+									onClick={onToggle}
+									aria-expanded={isOpen}
+									style={{
+										padding: '0 10px',
+										fill: '#FFF',
+										alignItems: 'center',
+										display: 'flex',
+									}}
+								>
+									<Icon icon={cog} />
+								</Button>
+							)}
+							renderContent={({ onClose }) => (
+								<div
+									style={{
+										width: 400,
+										maxWidth: '100vw',
+										padding: 0,
+									}}
+								>
+									<div className={css.headerSection}>
+										<h2 style={{ margin: 0 }}>
+											Playground settings
+										</h2>
+									</div>
+									<ActiveSiteSettingsForm
+										onSubmit={onClose}
+									/>
+								</div>
+							)}
+						/>
 						{activeSite?.metadata?.storage === 'local-fs' ? (
 							<SyncLocalFilesButton />
 						) : null}