-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Editor: Refine availability of rename post action #62248
Conversation
Size Change: +190 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
@@ -59,6 +59,42 @@ test.describe( 'Test Custom Post Types', () => { | |||
await expect( parentPageLocator ).toHaveValue( parentPage ); | |||
} ); | |||
|
|||
test( 'should not be able to rename a post that is either auto-drafted or lacks title support', async ( { |
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.
Adding this e2e test was the first thing that occurred to me but now I wonder if it’d be better to create unit tests for post-actions which currently lacks any.
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. |
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.
Thanks for the PR and the fix! This is something that should be included in 6.6.
I think it's a better approach to check if a post type supports title
here, similar to how we check for revisions support. The reason is that supports
is about all items of a post type, whereas isEligible
is more useful for per item
checks.
With the above approach, I think we don't need to also add the auto-draft
check in isEligible
function, correct?
Thanks for the 🎯 advice Nik! Checking supports on the post type is indeed the indicated remedy. I was asking myself why |
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.
Looks great, thank you!
Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
This was cherry-picked to the wp/6.6 branch. |
What?
Omits the "rename" action on posts without "title" support.
Why?
At present a post without "title" support has the action available but using it causes the editor to crash.
How?
Adds a condition to omit the action when the post type lacks title support.
Testing Instructions
If you don’t have a custom post type without title support handy there’s an e2e test added in the first commit that fails without the second commit. So from this branch with dev script running:
checkout HEAD~
npm run test:e2e -- custom-post-types
git switch -
Or manually:
Screenshots or screencast
Before this PR, the crash:
crash-on-rename.mp4