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.

Sign up for GitHub

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

Update site editor region labels to match post editor #42037

Merged
merged 4 commits into from
Jul 4, 2022
Merged

Update site editor region labels to match post editor #42037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60e62ec
Update site editor region labels to match post editor
talldan Jun 29, 2022
ddafc11
Update end to end tests to use new region labels in site editor
talldan Jun 30, 2022
2adc1a4
Update actions/publish region label
talldan Jul 4, 2022
81c8f9c
Update e2e test selector to use new region name
talldan Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions packages/e2e-test-utils-playwright/src/editor/site-editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,14 @@ import type { Editor } from './index';
* @param {Editor} this
*/
export async function saveSiteEditorEntities( this: Editor ) {
await this.page
.locator( 'role=region[name="Header"i] >> role=button[name="Save"i]' )
.click();

// The sidebar entities panel opens with another save button. Click this too.
await this.page
.locator( 'role=region[name="Publish"i] >> role=button[name="Save"i]' )
.click();

// The panel will close revealing the main editor save button again.
// It will have the classname `.is-busy` while saving. Wait for it to
// not have that classname.
// TODO - find a way to improve this selector to use role/name.
await this.page.click(
'role=region[name="Editor top bar"i] >> role=button[name="Save"i]'
);
// Second Save button in the entities panel.
await this.page.click(
'role=region[name="Editor publish"i] >> role=button[name="Save"i]'
);
await this.page.waitForSelector(
'css=.edit-site-save-button__button:not(.is-busy)'
'role=region[name="Editor top bar"i] >> role=button[name="Save"i][disabled]'
);
}
4 changes: 2 additions & 2 deletions packages/e2e-test-utils/src/site-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ export async function toggleGlobalStyles() {
* @param {string} panelName Name of the panel that is going to be opened.
*/
export async function openGlobalStylesPanel( panelName ) {
const selector = `//div[@aria-label="Settings"]//button[.//*[text()="${ panelName }"]]`;
const selector = `//div[@aria-label="Editor settings"]//button[.//*[text()="${ panelName }"]]`;
await ( await page.waitForXPath( selector ) ).click();
}

Expand All @@ -283,6 +283,6 @@ export async function openGlobalStylesPanel( panelName ) {
*/
export async function openPreviousGlobalStylesPanel() {
await page.click(
'div[aria-label="Settings"] button[aria-label="Navigate to the previous view"]'
'div[aria-label="Editor settings"] button[aria-label="Navigate to the previous view"]'
);
}
11 changes: 11 additions & 0 deletions packages/edit-site/src/components/editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ import { GlobalStylesProvider } from '../global-styles/global-styles-provider';
import useTitle from '../routes/use-title';

const interfaceLabels = {
/* translators: accessibility text for the editor top bar landmark region. */
header: __( 'Editor top bar' ),
/* translators: accessibility text for the editor content landmark region. */
body: __( 'Editor content' ),
/* translators: accessibility text for the editor settings landmark region. */
sidebar: __( 'Editor settings' ),
/* translators: accessibility text for the editor publish landmark region. */
actions: __( 'Editor publish' ),
/* translators: accessibility text for the editor footer landmark region. */
Copy link
Member

Choose a reason for hiding this comment

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

I think we should also update the actions landmark region label. Currently, it says "Publish".

gutenberg/packages/edit-post/src/components/layout/index.js

Lines 60 to 61 in f2888ed

/* translators: accessibility text for the editor publish landmark region. */
actions: __( 'Editor publish' ),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure about this one. My thinking was that 'publish' isn't quite the right term in the site editor, but I didn't realise the default is already 'publish' 😄 . I thought it would be 'Actions'.

Thanks for reviewing.

Copy link
Member

@Mamaduka Mamaduka Jul 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good point! I'm not sure what label will work for both editors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll keep it as 'Editor publish' for now as I don't have any better ideas.

footer: __( 'Editor footer' ),
/* translators: accessibility text for the navigation sidebar landmark region. */
drawer: __( 'Navigation Sidebar' ),
};

Expand Down
63 changes: 25 additions & 38 deletions test/e2e/specs/site-editor/template-revert.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

await page.click( 'role=button[name="Show template details"i]' );

Expand All @@ -57,9 +57,9 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

const contentAfter =
await templateRevertUtils.getCurrentSiteEditorContent();
Expand All @@ -78,9 +78,9 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await admin.visitSiteEditor();

const contentAfter =
Expand All @@ -97,20 +97,20 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
const contentBefore =
await templateRevertUtils.getCurrentSiteEditorContent();

// Revert template and check state.
await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
const contentAfterSave =
await templateRevertUtils.getCurrentSiteEditorContent();
expect( contentAfterSave ).not.toEqual( contentBefore );

// Undo revert by clicking header button and check state again.
await page.click(
'role=region[name="Header"i] >> role=button[name="Undo"i]'
'role=region[name="Editor top bar"i] >> role=button[name="Undo"i]'
);
const contentAfterUndo =
await templateRevertUtils.getCurrentSiteEditorContent();
Expand All @@ -126,12 +126,12 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
const contentBefore =
await templateRevertUtils.getCurrentSiteEditorContent();

await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

// Click the snackbar "Undo" button.
await page.click(
Expand All @@ -155,19 +155,19 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await page.click(
'role=region[name="Header"i] >> role=button[name="Undo"i]'
'role=region[name="Editor top bar"i] >> role=button[name="Undo"i]'
);

const contentAfterUndo =
await templateRevertUtils.getCurrentSiteEditorContent();
expect( contentAfterUndo ).not.toEqual( contentBefore );

await page.click(
'role=region[name="Header"i] >> role=button[name="Redo"i]'
'role=region[name="Editor top bar"i] >> role=button[name="Redo"i]'
);

const contentAfterRedo =
Expand All @@ -187,9 +187,9 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

// Click undo in the snackbar. This reverts revert template action.
await page.click(
Expand All @@ -203,7 +203,7 @@ test.describe( 'Template Revert', () => {

// Click undo again, this time in the header. Reverts initial dummy content.
await page.click(
'role=region[name="Header"i] >> role=button[name="Undo"i]'
'role=region[name="Editor top bar"i] >> role=button[name="Undo"i]'
);

// Check dummy content is gone.
Expand All @@ -222,18 +222,18 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
const contentBefore =
await templateRevertUtils.getCurrentSiteEditorContent();

await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

await page.click(
'role=region[name="Header"i] >> role=button[name="Undo"i]'
'role=region[name="Editor top bar"i] >> role=button[name="Undo"i]'
);

await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

await admin.visitSiteEditor();

Expand All @@ -252,18 +252,18 @@ test.describe( 'Template Revert', () => {
name: 'core/paragraph',
attributes: { content: 'Test' },
} );
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
const contentBefore =
await templateRevertUtils.getCurrentSiteEditorContent();

await templateRevertUtils.revertTemplate();
await templateRevertUtils.save();
await editor.saveSiteEditorEntities();

await page.click(
'role=button[name="Dismiss this notice"i] >> role=button[name="Undo"i]'
);

await templateRevertUtils.save();
await editor.saveSiteEditorEntities();
await admin.visitSiteEditor();

const contentAfter =
Expand All @@ -277,19 +277,6 @@ class TemplateRevertUtils {
this.page = page;
}

async save() {
await this.page.click(
'role=region[name="Header"i] >> role=button[name="Save"i]'
);
// Second Save button in the entities panel.
await this.page.click(
'role=region[name="Publish"i] >> role=button[name="Save"i]'
);
await this.page.waitForSelector(
'role=region[name="Header"i] >> role=button[name="Save"i][disabled]'
);
}

async revertTemplate() {
await this.page.click( 'role=button[name="Show template details"i]' );
await this.page.click( 'role=menuitem[name=/Clear customizations/i]' );
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/specs/site-editor/title.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ test.describe( 'Site editor title', () => {
} );

const title = await page.locator(
'role=region[name="Header"i] >> role=heading[level=1]'
'role=region[name="Editor top bar"i] >> role=heading[level=1]'
);

await expect( title ).toHaveText( 'Editing template: Index' );
Expand All @@ -40,7 +40,7 @@ test.describe( 'Site editor title', () => {
} );

const title = await page.locator(
'role=region[name="Header"i] >> role=heading[level=1]'
'role=region[name="Editor top bar"i] >> role=heading[level=1]'
);

await expect( title ).toHaveText( 'Editing template part: header' );
Expand Down