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

Add: Delete template action. #31678

Merged
merged 8 commits into from
May 19, 2021
Merged

Add: Delete template action. #31678

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
afa93c1
Add. Delete template action.
jorgefilipecosta May 10, 2021
b98025f
Update packages/edit-post/src/components/header/template-title/delete…
jorgefilipecosta May 19, 2021
ba756a5
Update packages/edit-post/src/components/header/template-title/delete…
jorgefilipecosta May 19, 2021
5880d2d
Update packages/edit-post/src/components/header/template-title/delete…
jorgefilipecosta May 19, 2021
a2deb75
Update packages/edit-post/src/components/header/template-title/delete…
jorgefilipecosta May 19, 2021
23a6e0e
Update packages/edit-post/src/components/header/template-title/index.js
jorgefilipecosta May 19, 2021
8def5f5
Update packages/edit-post/src/components/header/template-title/delete…
jorgefilipecosta May 19, 2021
ff30472
Add edit template name.
jorgefilipecosta May 19, 2021
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
84 changes: 84 additions & 0 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
@@ -0,0 +1,84 @@
/**
* External dependencies
*/
import { pickBy } from 'lodash';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
import { MenuGroup, MenuItem } 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';

/**
* Internal dependencies
*/
import { store as editPostStore } from '../../../store';

export default function DeleteTemplate() {
const { clearSelectedBlock } = useDispatch( blockEditorStore );
const { setIsEditingTemplate } = useDispatch( editPostStore );
const { getEditorSettings } = useSelect( editorStore );
const { updateEditorSettings, editPost } = useDispatch( editorStore );
const { deleteEntityRecord } = useDispatch( coreStore );
const { template } = useSelect( ( select ) => {
const { isEditingTemplate, getEditedPostTemplate } = select(
editPostStore
);
const _isEditing = isEditingTemplate();
return {
template: _isEditing ? getEditedPostTemplate() : null,
};
}, [] );

if ( ! template || ! template.wp_id ) {
return null;
}
return (
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
<MenuGroup className="edit-post-template-top-area__second-menu-group">
<MenuItem
isDestructive
isTertiary
isLink
onClick={ () => {
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
if (
// eslint-disable-next-line no-alert
window.confirm(
__(
'Are you sure you want to delete this template? It may be currently in use by other pages or posts.'
)
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
)
) {
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' ) }
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
</MenuItem>
</MenuGroup>
);
}
43 changes: 36 additions & 7 deletions packages/edit-post/src/components/header/template-title/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import {
Button,
NavigableMenu,
Dropdown,
MenuGroup,
MenuItem,
} from '@wordpress/components';

/**
* Internal dependencies
*/
import { store as editPostStore } from '../../../store';
import DeleteTemplate from './delete-template';

function TemplateTitle() {
const { template, isEditing } = useSelect( ( select ) => {
Expand All @@ -33,12 +41,33 @@ function TemplateTitle() {
}

return (
<span className="edit-post-template-title">
{
/* translators: 1: Template name. */
sprintf( __( 'Editing template: %s' ), templateTitle )
}
</span>
<Dropdown
position="bottom center"
className="edit-post-template-top-area"
contentClassName="edit-post-template-top-area__popover"
renderToggle={ ( { onToggle } ) => (
<>
<div className="edit-post-template-title">
{ __( 'About' ) }
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
</div>
<Button isSmall isTertiary onClick={ onToggle }>
jorgefilipecosta marked this conversation as resolved.
Show resolved Hide resolved
{ templateTitle }
Copy link
Contributor

@carolinan carolinan May 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What is a suitable context for this button?
What do we call the modal popover, "Template options"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes the aria-label sounds good to me 👍

</Button>
</>
) }
renderContent={ () => (
<NavigableMenu>
Copy link
Contributor

@talldan talldan May 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we want to achieve something like the example:
image

I think it's probably better to avoid using a NavigableMenu/MenuGroup/MenuItem, which should generally only be used for vertical list menus. Those components are not needed in a plain dropdown and will make it difficult to achieve the desired result. I think the screenshot above is closer to something like the Link UI in terms of the way it's presented. Arrow key navigation isn't needed for that kind of component, tabs are fine.

An example is here where the Delete button takes up the full width because it's a menu item (and some of the other styles aren't quite right):
Screenshot 2021-05-19 at 5 44 43 pm

Copy link
Contributor

Choose a reason for hiding this comment

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

Although I see in the issue that this is the latest quick win design:
Screenshot 2021-04-22 at 12 17 03

I still think it'd be better not to use NavigableMenu because there's only one button that can be tabbed to.

Copy link
Contributor

Choose a reason for hiding this comment

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

A close button needs to be added because it is now a keyboard trap.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, there are some other designs proposed in this PR as well, not sure which is the latest 😄

A close button needs to be added because it is now a keyboard trap.

Escape or clicking outside should be fine, most popovers in the editor do this.

<MenuGroup
className="edit-post-template-top-area__first-menu-group"
label={ __( 'Title' ) }
disabled
>
<MenuItem>{ templateTitle }</MenuItem>
</MenuGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

This menu is clickable and focusable but doesn't actually do anything.
I suggest not adding this as a menu until there is an action.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, the user already clicked on a button with the template name, so making this focusable doesn't add anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned here, this should be an input, not a menu item.

<DeleteTemplate />
</NavigableMenu>
) }
/>
);
}

Expand Down
24 changes: 24 additions & 0 deletions packages/edit-post/src/components/header/template-title/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,27 @@
flex-grow: 1;
justify-content: center;
}

.edit-post-template-top-area {
display: flex;
flex-direction: column;
align-content: space-between;
width: 100%;
align-items: center;
}

.edit-post-template-top-area__popover .components-popover__content {
min-width: 360px;
}

.edit-post-template-top-area__popover.components-dropdown__content .components-popover__content > div {
padding: 0;
}

.edit-post-template-top-area__first-menu-group {
padding: $grid-unit-15 $grid-unit-15 0 $grid-unit-15;
}

.edit-post-template-top-area__second-menu-group {
padding: $grid-unit-15;
}