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

Migrate Post Template Delete button from confirm() to ConfirmDialog #37535

Merged
merged 11 commits into from
Feb 22, 2022
Merged

Migrate Post Template Delete button from confirm() to ConfirmDialog #37535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9e20b87
migrate post template menu item from `confirm()` to `ConfirmDialog`
chad1008 Dec 17, 2021
4a4c836
migrate the `Delete template` menu item from `confirm()` to `ConfirmD…
chad1008 Dec 20, 2021
e0eb256
simplify ConfirmDialog state using the built in isOpen prop
chad1008 Jan 7, 2022
edc02bd
explicitly close ConfirmDialog after confirmation
chad1008 Jan 21, 2022
a328a83
add e2e tests for Delete Template ConfirmDialog
chad1008 Feb 7, 2022
17a2894
replace editor animation that was removed for local testing
chad1008 Feb 7, 2022
ec2d19b
implement new `deleteAllTemplates()` util
chad1008 Feb 15, 2022
34d2c45
simplify xpath for increased resiliancy when switching to template ed…
chad1008 Feb 15, 2022
bf59df0
save and reload post to ensure template dropdown is properly updated
chad1008 Feb 16, 2022
c79ff28
replace an unneeded reload with a more user-relatable button click
chad1008 Feb 16, 2022
415ef76
ensure small viewports are able to open/close settings panel correctly
chad1008 Feb 16, 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
181 changes: 179 additions & 2 deletions packages/e2e-tests/specs/editor/various/post-editor-template-mode.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
activatePlugin,
deactivatePlugin,
deleteAllTemplates,
setBrowserViewport,
} from '@wordpress/e2e-test-utils';

const openSidebarPanelWithTitle = async ( title ) => {
Expand Down Expand Up @@ -56,7 +57,7 @@ const switchToTemplateMode = async () => {

// Check that we switched properly to edit mode.
await page.waitForXPath(
'//*[contains(@class, "components-snackbar")]/*[text()="Editing template. Changes made here affect all posts and pages that use the template."]'
'//*[text()="Editing template. Changes made here affect all posts and pages that use the template."]'
);
const title = await page.$eval(
'.edit-post-template-top-area',
Expand Down Expand Up @@ -119,7 +120,6 @@ describe( 'Post Editor Template mode', () => {
// there's a template resolution bug forcing us to do so.
await saveDraft();
await page.reload();

await switchToTemplateMode();

// Edit the template
Expand Down Expand Up @@ -197,3 +197,180 @@ describe( 'Post Editor Template mode', () => {
expect( content ).toMatchSnapshot();
} );
} );

describe( 'Delete Post Template Confirmation Dialog', () => {
beforeAll( async () => {
await activatePlugin( 'gutenberg-test-block-templates' );
await deleteAllTemplates( 'wp_template' );
await deleteAllTemplates( 'wp_template_part' );
await activateTheme( 'twentytwentyone' );
await createNewPost();
// Create a random post.
await page.type( '.editor-post-title__input', 'Just an FSE Post' );
await page.keyboard.press( 'Enter' );
await page.keyboard.type( 'Hello World' );

// Save the post
// Saving shouldn't be necessary but unfortunately,
// there's a template resolution bug forcing us to do so.
await saveDraft();
await page.reload();
// Unselect the blocks.
await page.evaluate( () => {
wp.data.dispatch( 'core/block-editor' ).clearSelectedBlock();
} );
} );

afterAll( async () => {
await activateTheme( 'twentytwentyone' );
await deactivatePlugin( 'gutenberg-test-block-templates' );
} );

[ 'large', 'small' ].forEach( ( viewport ) => {
it( `should retain template if deletion is canceled when the viewport is ${ viewport }`, async () => {
await setBrowserViewport( viewport );

const isWelcomeGuideActive = await page.evaluate( () =>
wp.data
.select( 'core/edit-post' )
.isFeatureActive( 'welcomeGuide' )
);
if ( isWelcomeGuideActive === true ) {
await page.evaluate( () =>
wp.data
.dispatch( 'core/edit-post' )
.toggleFeature( 'welcomeGuide' )
);
await page.reload();
await page.waitForSelector( '.edit-post-layout' );
}
if ( viewport === 'small' ) {
await page.waitForXPath( '//button[@aria-label="Settings"]' );
await openDocumentSettingsSidebar();
}
const templateTitle = `${ viewport } Viewport Deletion Test`;

await createNewTemplate( templateTitle );
// Edit the template
if ( viewport === 'small' ) {
await page.waitForXPath( `//h2[text()="${ templateTitle }"]` );
const closeDocumentSettingsButton = await page.waitForXPath(
'//button[@aria-label="Close settings"]'
);
await closeDocumentSettingsButton.click();
}
await insertBlock( 'Paragraph' );
await page.keyboard.type(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous test in the suite leaves the site on Twenty Twenty-one, which works well for the tests I've added. I did notice, however that switching to another theme (tested Twenty Twenty-two and Emptytheme) causes the new tests to fail (see line 206 for the e2e-util that handles the theme switch)

On either of those two themes, and potentially others, this call to page.keyboard.type isn't able to complete. It stops midway through the string for reasons I can't see and the test ends up failing.

'Just a random paragraph added to the template'
);

// Save changes
const publishButton = await page.waitForXPath(
`//button[contains(text(), 'Publish')]`
);
await publishButton.click();
const saveButton = await page.waitForXPath(
`//div[contains(@class, "entities-saved-states__panel-header")]/button[contains(text(), 'Save')]`
);
await saveButton.click();
// Avoid publishing the post
// Select the cancel button via parent div's class, because the text `Cancel` is used on another button as well
const cancelButton = await page.waitForXPath(
`//div[contains(@class,"editor-post-publish-panel__header-cancel-button")]/button[not(@disabled)]`
);
await cancelButton.click();

const templateDropdown = await page.waitForXPath(
`//button[contains(text(), '${ templateTitle }')]`
);
await templateDropdown.click();
const deleteTemplateButton = await page.waitForXPath(
'//button[@role="menuitem"][@aria-label="Delete template"]'
);
await deleteTemplateButton.click();

await page.waitForXPath(
`//*[text()="Are you sure you want to delete the ${ templateTitle } template? It may be used by other pages or posts."]`
);
const dialogCancelButton = await page.waitForXPath(
'//*[@role="dialog"][not(@id="wp-link-wrap")]//button[text()="Cancel"]'
);
await dialogCancelButton.click();

const exitTemplateModeButton = await page.waitForXPath(
'//button[text()="Back"]'
);
await exitTemplateModeButton.click();

await page.waitForXPath(
'//button[@aria-label="Settings"][@aria-expanded="false"]'
);
await openDocumentSettingsSidebar();

const element = await page.waitForXPath(
'//h2/button[contains(text(), "Template")]/../..//select'
);
const value = await element.getProperty( 'value' );
const currentTemplateSlug = await value.jsonValue();

expect( currentTemplateSlug ).toBe(
`wp-custom-template-${ viewport }-viewport-deletion-test`
);
} );

it( `should delete template if deletion is confirmed when the viewport is ${ viewport }`, async () => {
const templateTitle = `${ viewport } Viewport Deletion Test`;

await setBrowserViewport( viewport );

await switchToTemplateMode();
if ( viewport === 'small' ) {
const closeDocumentSettingsButton = await page.waitForXPath(
'//div[contains(@class,"interface-complementary-area-header__small")]/button[@aria-label="Close settings"]'
);
await closeDocumentSettingsButton.click();
}

const templateDropdown = await page.waitForXPath(
`//button[contains(text(), '${ templateTitle }')]`
);
await templateDropdown.click();

const deleteTemplateButton = await page.waitForXPath(
'//button[@role="menuitem"][@aria-label="Delete template"]'
);
await deleteTemplateButton.click();

await page.waitForXPath(
`//*[text()="Are you sure you want to delete the ${ templateTitle } template? It may be used by other pages or posts."]`
);
const dialogConfirmButton = await page.waitForXPath(
'//*[@role="dialog"][not(@id="wp-link-wrap")]//button[text()="OK"]'
);

await dialogConfirmButton.click();

// Saving isn't technically necessary, but for themes without any specified templates,
// the removal of the Templates dropdown is delayed. A save and reload allows for this
// delay and prevents flakiness
await saveDraft();
await page.reload();

const optionElementHandlers = await page.$x(
'//h2/button[contains(text(), "Template")]/../..//select/option'
);
const availableTemplates = [];
for ( const elem of optionElementHandlers ) {
const elemName = await elem.getProperty( 'textContent' );
const templateName = await elemName.jsonValue();
availableTemplates.push( templateName );
}

expect(
availableTemplates.includes(
`${ viewport } Viewport Deletion Test`
)
).toBe( false );
} );
} );
} );
103 changes: 57 additions & 46 deletions packages/edit-post/src/components/header/template-title/delete-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ import { pickBy } from 'lodash';
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { MenuGroup, MenuItem } from '@wordpress/components';
import {
MenuGroup,
MenuItem,
__experimentalConfirmDialog as ConfirmDialog,
} from '@wordpress/components';
import { store as blockEditorStore } from '@wordpress/block-editor';
import { useDispatch, useSelect } from '@wordpress/data';
import { store as editorStore } from '@wordpress/editor';
import { store as coreStore } from '@wordpress/core-data';
import { useState } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -33,6 +38,7 @@ export default function DeleteTemplate() {
template: _isEditing ? getEditedPostTemplate() : null,
};
}, [] );
const [ showConfirmDialog, setShowConfirmDialog ] = useState( false );

if ( ! template || ! template.wp_id ) {
return null;
Expand All @@ -42,53 +48,58 @@ export default function DeleteTemplate() {
templateTitle = template.title;
}

const onDelete = () => {
clearSelectedBlock();
setIsEditingTemplate( false );
setShowConfirmDialog( false );

editPost( {
template: '',
} );
const settings = getEditorSettings();
const newAvailableTemplates = pickBy(
settings.availableTemplates,
( _title, id ) => {
return id !== template.slug;
}
);
updateEditorSettings( {
...settings,
availableTemplates: newAvailableTemplates,
} );
deleteEntityRecord( 'postType', 'wp_template', template.id );
};

return (
<MenuGroup className="edit-post-template-top-area__second-menu-group">
<MenuItem
className="edit-post-template-top-area__delete-template-button"
isDestructive
variant="secondary"
aria-label={ __( 'Delete template' ) }
onClick={ () => {
if (
// eslint-disable-next-line no-alert
window.confirm(
sprintf(
/* translators: %s: template name */
__(
'Are you sure you want to delete the %s template? It may be used by other pages or posts.'
),
templateTitle
)
)
) {
clearSelectedBlock();
setIsEditingTemplate( false );

editPost( {
template: '',
} );
const settings = getEditorSettings();
const newAvailableTemplates = pickBy(
settings.availableTemplates,
( _title, id ) => {
return id !== template.slug;
}
);
updateEditorSettings( {
...settings,
availableTemplates: newAvailableTemplates,
} );
deleteEntityRecord(
'postType',
'wp_template',
template.id
);
}
} }
>
{ __( 'Delete template' ) }
</MenuItem>
<>
<MenuItem
className="edit-post-template-top-area__delete-template-button"
isDestructive
variant="secondary"
aria-label={ __( 'Delete template' ) }
onClick={ () => {
setShowConfirmDialog( true );
} }
ciampo marked this conversation as resolved.
Show resolved Hide resolved
>
{ __( 'Delete template' ) }
</MenuItem>
<ConfirmDialog
isOpen={ showConfirmDialog }
onConfirm={ onDelete }
onCancel={ () => {
setShowConfirmDialog( false );
} }
ciampo marked this conversation as resolved.
Show resolved Hide resolved
>
{ sprintf(
/* translators: %s: template name */
__(
'Are you sure you want to delete the %s template? It may be used by other pages or posts.'
),
templateTitle
) }
</ConfirmDialog>
</>
</MenuGroup>
);
}