Consolidate and fix rename post action
#61857
Merged
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
ntsekouras
added
[Type] Regression
ntsekouras
requested review from
mcsf,
Mamaduka,
youknowriad and
jorgefilipecosta
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -96 B (-0.01%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
ntsekouras
force-pushed
the
consolidate-and-fix-rename-post-action
branch
from
d7c8253
to
bf63d44
Compare
|
I like this approach. |
Mamaduka
reviewed
May 22, 2024
|
Flaky tests detected in 9c4b3cb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9191364525
|
Mamaduka
approved these changes
May 22, 2024
There was a problem hiding this comment.
Thanks, Nik!
The fix tests well for me ✅
Also, while testing this fix, I noticed that theme patterns (PATTERN_TYPES.theme) have "Edit" and "Move to Trash" actions available in DataViews. These actions do nothing and should probably only be available for user patterns.
|
|
Mamaduka
reviewed
May 22, 2024
Comment on lines
+1126
to
+1129
| isTemplateOrTemplatePart ? resetTemplateAction : restorePostAction, | ||
| isTemplateOrTemplatePart | ||
| ? deleteTemplateAction | ||
| : permanentlyDeletePostAction, |
There was a problem hiding this comment.
Sidenote: The way we build the actions array here is difficult to parse. Maybe we should convert it into separate if conditions.
Example:
if ( condition ) {
actions.push( action );
}There was a problem hiding this comment.
The order matters for UI for now and would need multiple if conditions. So I didn't do it here, even though I thought about it too. Will check it in follow ups where more actions will be consolidated.
There was a problem hiding this comment.
It's definitely not a blocker, just something that comes to my mind every time I see it 😄
I agree. We should have a flag in REST response.
Yeah, I'll handle them in follow ups. |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
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.
What?
Part of: #61787
After a recent refactor to post actions some regressions were introduced, where we show more actions than we should in certain cases. An example is the
renamepost action which has different conditions based on some core post types (like templates, template parts, etc..). Additionally the goal is to use a single action for all post types where we can, likerename, delete, etc...This PR consolidates the
renameaction and adds all needed checks for core post types. In the future when we'll have an API for third party devs to register actions, it should be possible to also remove core actions too. This way they would be able to implement a custom rename action if they want..I've changed only one action here to discuss the approach and if deemed fine, I'll update more either here or in follow ups.
Testing Instructions
renameaction works as expected and is shown only when it should. For example it shouldn't be shown in theme patterns or templates or template parts.