Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Oct 7, 2024
2 changes: 1 addition & 1 deletion packages/playground/website/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
</script>
</head>
<body>
<main id="root">
<main id="root" aria-label="WordPress Playground">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love how addressing elements of a page for E2E tests has overlapping concerns with A11Y (or maybe it is better said the other way around A11Y -> E2E-testability)

<script>
const query = new URLSearchParams(window.location.search);
const shouldLazyLoadPlayground =
Expand Down
158 changes: 92 additions & 66 deletions packages/playground/website/playwright/e2e/website-ui.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/'
);
Expand All @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good for our WordPress builds to encode minimum supported PHP versions and at least warn about incompatible version in UI. It looks like the WordPress.org API does list the minimum supported PHP version for each release. From
https://api.wordpress.org/core/version-check/1.7/?channel=beta :

{
	"response": "development",
	"download": "https:\/\/downloads.wordpress.org\/release\/wordpress-6.7-beta1.zip",
	"locale": "en_US",
	"packages": {
		"full": "https:\/\/downloads.wordpress.org\/release\/wordpress-6.7-beta1.zip",
		"no_content": false,
		"new_bundled": false,
		"partial": false,
		"rollback": false
	},
	"current": "6.7-beta1",
	"version": "6.7-beta1",
	"php_version": "7.2.24",
	"mysql_version": "5.5.5",
	"new_bundled": "6.4",
	"partial_version": false
}

If we had that info, we could stop hardcoding PHP versions here.

Created an issue for adding the info:
#1857

* @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();
});
});

Expand All @@ -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);
});
});

Expand All @@ -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 ({
Expand Down
75 changes: 4 additions & 71 deletions packages/playground/website/playwright/website-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 })) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. :) I didn't know about isHidden().

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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
Loading
Loading