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

Jump to bottom

Reuse and unify post and page actions, accross the different use cases. #59849

Closed
wants to merge 11 commits into from
Closed

Reuse and unify post and page actions, accross the different use cases. #59849

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
9b24862
Add: Reusable post actions.
jorgefilipecosta Mar 12, 2024
431b77b
in progress
jorgefilipecosta Mar 12, 2024
ae011ed
reuse actions on details view
jorgefilipecosta Mar 13, 2024
5340396
working across different actions uses.
jorgefilipecosta Mar 13, 2024
d92a6b1
in progress
jorgefilipecosta Mar 13, 2024
fe078b0
fix onPerform
jorgefilipecosta Mar 13, 2024
1464197
use actions on data views too
jorgefilipecosta Mar 13, 2024
32a9320
code quality enhacements
jorgefilipecosta Mar 13, 2024
6116b91
lint fixes
jorgefilipecosta Mar 13, 2024
3248418
hasEdit
jorgefilipecosta Mar 14, 2024
2f87575
Update packages/editor/src/components/post-actions/utility-components.js
jorgefilipecosta Mar 21, 2024
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
1 change: 1 addition & 0 deletions packages/base-styles/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ $z-layers: (
".block-editor-block-rename-modal": 1000001,
".edit-site-list__rename-modal": 1000001,
".dataviews-action-modal": 1000001,
".editor-action-modal": 1000001,
".editor-post-template__swap-template-modal": 1000001,
".edit-site-template-panel__replace-template-modal": 1000001,

Expand Down
2 changes: 2 additions & 0 deletions packages/edit-post/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,12 @@
"@wordpress/core-commands": "file:../core-commands",
"@wordpress/core-data": "file:../core-data",
"@wordpress/data": "file:../data",
"@wordpress/date": "file:../date",
"@wordpress/deprecated": "file:../deprecated",
"@wordpress/dom": "file:../dom",
"@wordpress/editor": "file:../editor",
"@wordpress/element": "file:../element",
"@wordpress/html-entities": "file:../html-entities",
"@wordpress/hooks": "file:../hooks",
"@wordpress/i18n": "file:../i18n",
"@wordpress/icons": "file:../icons",
Expand Down
126 changes: 126 additions & 0 deletions packages/edit-post/src/components/sidebar/post-initial-panel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/**
* WordPress dependencies
*/
import {
PanelBody,
__experimentalText as Text,
__experimentalVStack as VStack,
} from '@wordpress/components';
import {
store as editorStore,
privateApis as editorPrivateApis,
} from '@wordpress/editor';
import { decodeEntities } from '@wordpress/html-entities';
import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';
import { page as pageIcon } from '@wordpress/icons';
import { __, sprintf } from '@wordpress/i18n';
import { humanTimeDiff } from '@wordpress/date';
import { addQueryArgs } from '@wordpress/url';

/**
* Internal dependencies
*/
import { unlock } from '../../../lock-unlock';

const {
PostActions: EditorPostActions,
postManagementActions,
PostSidebarCard,
} = unlock( editorPrivateApis );

const {
trashPostAction,
usePermanentlyDeletePostAction,
useRestorePostAction,
postRevisionsAction,
} = postManagementActions;

function PostActions( { postType, postId } ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is specific about edit-post that is forcing us to override the default "editor" PostActions?

Copy link
Member Author

Choose a reason for hiding this comment

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

When we execute an action, it may require us to navigate to a different page. For example, when we delete a post while editing it, we need to navigate to a completely different page using "window.location.href". This is a unique requirement on the edit-post. We are not replacing the action, but rather, we are adding specific callbacks to the onPerform function when needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

But it's almost always the same kind of override. So rather than all of this complex code, some simple onNavigateToList/onNavigateToPost props or something like that could work.

Copy link
Member Author

Choose a reason for hiding this comment

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

But it's almost always the same kind of override. So rather than all of this complex code, some simple onNavigateToList/onNavigateToPost props or something like that could work.

That would work. Thank you for the suggestion @youknowriad 👍 Another possibility is a a generic usePostActions( onPerform ) with a global onPerform( actionId, items) that receives the actionId and the items where the action was executed. And then the usage would be very simple:

usePostActions( onPerform( actionId, items)=> {
switch(actionId)
case: moveToTrass
case: permanentlyDelete:
onNavigateToList();
break:
case: editPost:
navigateToPost
} );

Let me know which API would you prefer a onNavigateToList and a onNavigateToPost or a global onPerform.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the generic onPerform is slightly better.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • onPerform is not very obvious. Maybe onNavigateAfterAction? onAfterAction? onActionPerformed?
  • I wonder whether that API is too powerful, though. Concerns:
    • For the reader, how obvious is it that this prop is meant to strictly set whether and where to route after the action is completed?
    • Is there a clearer and stricter alternative?
    • Does this introduce indirection, in the sense that the reader needs to juggle two distinct components to understand the whole cascade of effects?
    • (This is not a strong opinion, just caution.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good points. The onPerform may be used for other things besides navigation e.g: close a menu after the action is performed etc. And we already had onPerform for actions before:

gutenberg/packages/edit-site/src/components/actions/index.js

Lines 150 to 153 in 9c0a5ac

if ( onPerform ) {
onPerform();
}
closeModal();

But not all of the actions had it.

I think I will go for onActionPerformed, it more clear on what it does and what is expected when compared with just onPerform.

const permanentlyDeletePostAction = usePermanentlyDeletePostAction();
const restorePostAction = useRestorePostAction();

const actions = useMemo( () => {
const TrashPostModal = trashPostAction.RenderModal;
const customizedTrashPostAction = {
...trashPostAction,
RenderModal: ( props ) => {
return (
<TrashPostModal
{ ...props }
onPerform={ () => {
if ( props.onPerform ) {
props.onPerform();
}
Comment on lines +52 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for fun:

Suggested change
if ( props.onPerform ) {
props.onPerform();
}
props.onPerform?.();

window.location.href = addQueryArgs( 'edit.php', {
post_type: postType,
} );
} }
/>
);
},
};
const customizedPermanentlyDeletePostAction = {
...permanentlyDeletePostAction,
callback: async ( posts, onPerform ) => {
await permanentlyDeletePostAction.callback( posts, () => {
if ( onPerform ) {
onPerform();
}
window.location.href = addQueryArgs( 'edit.php', {
post_type: postType,
} );
} );
},
};
return [
customizedTrashPostAction,
customizedPermanentlyDeletePostAction,
restorePostAction,
postRevisionsAction,
];
}, [ permanentlyDeletePostAction, restorePostAction, postType ] );

return (
<EditorPostActions
postType={ postType }
postId={ postId }
actions={ actions }
/>
);
}

export default function PostInitialSidebar() {
const { postType, postId, modified, title } = useSelect( ( select ) => {
const { getCurrentPostType, getCurrentPostId, getEditedPostAttribute } =
select( editorStore );
return {
postType: getCurrentPostType(),
postId: getCurrentPostId(),
title: getEditedPostAttribute( 'title' ),
modified: getEditedPostAttribute( 'modified' ),
};
} );
return (
<PanelBody>
<PostSidebarCard
title={ decodeEntities( title ) }
icon={ pageIcon }
actions={
<PostActions postType={ postType } postId={ postId } />
}
description={
<VStack>
<Text>
{ sprintf(
// translators: %s: Human-readable time difference, e.g. "2 days ago".
__( 'Last edited %s' ),
humanTimeDiff( modified )
) }
</Text>
</VStack>
}
/>
</PanelBody>
);
}
2 changes: 2 additions & 0 deletions packages/edit-post/src/components/sidebar/settings-sidebar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
*/
import SettingsHeader from '../settings-header';
import PostStatus from '../post-status';
import PostInitialSidebar from '../post-initial-panel';
import MetaBoxes from '../../meta-boxes';
import PluginDocumentSettingPanel from '../plugin-document-setting-panel';
import PluginSidebarEditPost from '../plugin-sidebar';
Expand Down Expand Up @@ -113,6 +114,7 @@ const SidebarContent = ( {
<Tabs.TabPanel tabId={ sidebars.document } focusable={ false }>
{ ! isEditingTemplate && (
<>
<PostInitialSidebar />
<PostStatus />
<PluginDocumentSettingPanel.Slot />
<PostLastRevisionPanel />
Expand Down
30 changes: 0 additions & 30 deletions packages/edit-site/src/components/page-actions/index.js
View file
Open in desktop

This file was deleted.

63 changes: 0 additions & 63 deletions packages/edit-site/src/components/page-actions/trash-page-menu-item.js
View file
Open in desktop

This file was deleted.

28 changes: 21 additions & 7 deletions packages/edit-site/src/components/page-pages/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { dateI18n, getDate, getSettings } from '@wordpress/date';
import { privateApis as routerPrivateApis } from '@wordpress/router';
import { useSelect, useDispatch } from '@wordpress/data';
import { DataViews } from '@wordpress/dataviews';
import { privateApis as editorPrivateApis } from '@wordpress/editor';

/**
* Internal dependencies
Expand All @@ -34,17 +35,19 @@ import {
OPERATOR_IS_NONE,
} from '../../utils/constants';

import {
import AddNewPageModal from '../add-new-page';
import Media from '../media';
import { unlock } from '../../lock-unlock';

const { postManagementActions } = unlock( editorPrivateApis );
const {
trashPostAction,
usePermanentlyDeletePostAction,
useRestorePostAction,
postRevisionsAction,
viewPostAction,
useEditPostAction,
} from '../actions';
import AddNewPageModal from '../add-new-page';
import Media from '../media';
import { unlock } from '../../lock-unlock';
postRevisionsAction,
} = postManagementActions;

const { useLocation, useHistory } = unlock( routerPrivateApis );

Expand Down Expand Up @@ -352,13 +355,24 @@ export default function PagePages() {
const permanentlyDeletePostAction = usePermanentlyDeletePostAction();
const restorePostAction = useRestorePostAction();
const editPostAction = useEditPostAction();
const customizedEditPostAction = {
...editPostAction,
callback: ( posts ) => {
const post = posts[ 0 ];
history.push( {
postId: post.id,
postType: post.type,
canvas: 'edit',
} );
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we have some "commands" that either navigate using the url or use history depending on whether we detect that we're in the site editor. Can't we do the same thing within the shared action and avoid overriding it entirely?

Copy link
Member Author

Choose a reason for hiding this comment

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

The action itself does not determine whether navigation is necessary or not. For instance, on data views (such as the posts list), the action is used as it is, without requiring any navigation. It is the specific location where the action is utilized that determines whether performing an action has a side effect, such as navigation.

The commands always navigate to the site editor, even if the post was in the post editor:

				const targetUrl = addQueryArgs( 'site-editor.php', args );
				if ( isSiteEditor ) {
					history.push( args );
				} else {
					document.location = targetUrl;
				}

When we are in the post editor and a post is deleted we want to go back to the "old post" list "edit.php?post_type=page" at least for now. We may also need to go to "edit.php?post_type=post" or to "edit.php?post_type=custom_post_type" so we don't want to go to the site editor every time like in commands.

We could have in the editor something like

usePostActions(withNavigation =true/false)

Where
usePostActions(false) is used on dataviews.

usePostActions(true) is used on post editor, site editor details and site editor inspector.

When it is true we would have some logic to check if we are in the site editor isSiteEditor(). If yes we navigate in a way if not we navigate in another way. But that approach seems wrong to me it is editor package that is implementing the specific logic of post-edit, and site-edit while to me it seems it should be the other way around.

},
};
const actions = useMemo(
() => [
viewPostAction,
trashPostAction,
restorePostAction,
permanentlyDeletePostAction,
editPostAction,
customizedEditPostAction,
postRevisionsAction,
],
[ permanentlyDeletePostAction, restorePostAction, editPostAction ]
Expand Down
Loading
Loading