-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
DataViews: Allow actions to be provided declaratively as a prop #55192
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useDispatch } from '@wordpress/data'; | ||
import { decodeEntities } from '@wordpress/html-entities'; | ||
import { store as coreStore } from '@wordpress/core-data'; | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
import { store as noticesStore } from '@wordpress/notices'; | ||
import { useMemo } from '@wordpress/element'; | ||
|
||
export default function useMoveToTrashAction() { | ||
const { createSuccessNotice, createErrorNotice } = | ||
useDispatch( noticesStore ); | ||
const { deleteEntityRecord } = useDispatch( coreStore ); | ||
|
||
return useMemo( | ||
() => ( { | ||
id: 'move-to-trash', | ||
label: __( 'Move to Trash' ), | ||
youknowriad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
async perform( post ) { | ||
try { | ||
await deleteEntityRecord( | ||
'postType', | ||
post.type, | ||
post.id, | ||
{}, | ||
{ throwOnError: true } | ||
); | ||
createSuccessNotice( | ||
sprintf( | ||
/* translators: The page's title. */ | ||
__( '"%s" moved to the Trash.' ), | ||
decodeEntities( post.title.rendered ) | ||
), | ||
{ | ||
type: 'snackbar', | ||
id: 'edit-site-page-trashed', | ||
} | ||
); | ||
} catch ( error ) { | ||
const errorMessage = | ||
error.message && error.code !== 'unknown_error' | ||
? error.message | ||
: __( | ||
'An error occurred while moving the page to the trash.' | ||
); | ||
|
||
createErrorNotice( errorMessage, { type: 'snackbar' } ); | ||
} | ||
}, | ||
isDesctructive: true, | ||
youknowriad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} ), | ||
[ createSuccessNotice, createErrorNotice, deleteEntityRecord ] | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,17 +10,11 @@ import { moreVertical } from '@wordpress/icons'; | |
*/ | ||
import TrashPageMenuItem from './trash-page-menu-item'; | ||
|
||
export default function PageActions( { | ||
postId, | ||
className, | ||
toggleProps, | ||
onRemove, | ||
} ) { | ||
export default function PageActions( { postId, toggleProps, onRemove } ) { | ||
return ( | ||
<DropdownMenu | ||
icon={ moreVertical } | ||
label={ __( 'Actions' ) } | ||
className={ className } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not used in any of the usage of PageActions. a bit of cleanup :) |
||
toggleProps={ toggleProps } | ||
> | ||
{ () => ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the actions would only be responsible for interacting with the
data
(remove, edit, etc.) and they'd have success and failure callbacks for the UI to handle them as it wishes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard for me at this point to know what actions are going to be about. I'm guessing actions can be anything that takes the current "item" as argument and do anything:
I think it's very hard to predict what actions would want to have access to and what flows they will trigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree -- I think it makes sense to let each action handle the complete process (including showing/changing UI) for the time being until more patterns are recognized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actions will need to do more and in follow ups we will probably need to change
label
to be a component to be able to render modals, etc..