DataViews: Allow actions to be provided declaratively as a prop #55192
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
[Type] Code Quality
ntsekouras
reviewed
Oct 10, 2023
Interesting, I wonder if this should be a dedicated reusable component that the data views package provide or if it's something custom. I think for a start, I'll go with custom component (just leave the existing one), but it's going to be interesting to see how that maps to the "actions" in the Data views long term. |
|
There's some duplication right now but I think it's better to start with duplication for this menu for now. |
|
Size Change: +430 B (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
|
Any ✅ here |
|
Is this working for you? This is what I see: Gravacao.do.ecra.2023-10-10.as.18.53.26.mov |
oandregal
reviewed
Oct 10, 2023
| import { useMemo } from '@wordpress/element'; | ||
|
|
||
| export default function useMoveToTrashAction() { | ||
| const { createSuccessNotice, createErrorNotice } = |
There was a problem hiding this comment.
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.
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:
- Export
- Duplicate
- Edit
- Open modals (rename)
- Download
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.
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.
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..
oandregal
reviewed
Oct 10, 2023
| return ( | ||
| <DropdownMenu | ||
| icon={ moreVertical } | ||
| label={ __( 'Actions' ) } | ||
| className={ className } |
There was a problem hiding this comment.
It seems PageActions with the className property was introduced a while ago at 375b16f Probably best to leave the component as it is?
There was a problem hiding this comment.
It's not used in any of the usage of PageActions. a bit of cleanup :)
|
@oandregal Good catch, I fixed the issue. I had used "postType" instead of "type" :) |
ntsekouras
approved these changes
Oct 11, 2023
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related #55083
What?
This is a small refactoring/API PR that extracts the "actions" property of the data views into a dedicated prop instead of leaving it as part of "fields".
The reasoning is simple:
Testing Instructions