-
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
Template Part: Hide Advanced panel for non-admin users #64721
Conversation
packages/block-library/src/template-part/edit/advanced-controls.js
Outdated
Show resolved
Hide resolved
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: -8 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
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 good to me ✅
* Template Part: Hide Advanced panel for non-admin users * Restore original conditional statement Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
hasInnerBlocks={ hasInnerBlocks } | ||
/> | ||
</InspectorControls> | ||
{ canUserEdit && ( |
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.
There's something that feels off to me here: why are we checking the user rights to edit template part when these changes won't be made in the template part object? they will be made in the container (whether that is a post or template is unknown here)
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.
Come to think of it, you are right.
Does this mean that we need additional code like the following to check if the template (not the template part) can be edited?
const canUserEditTemplates = select( coreStore ).canUser( 'create', {
kind: 'postType',
name: 'wp_template',
} );
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.
Not really IMO, it feels like a symptom of something else. Do you know why the block is selectable in the first place here? I mean if you can't edit a template, why should you be able to select a block within that template?
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.
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.
Perhaps should we make some changes to the code changed in #60010 to prevent non-admin users from selecting template parts?
That would be a good intermediary step but to be honest I never liked the fact that you are able to select template parts in "page focus mode", it feels contradictory to me and I'm not sure about the UX improvement. cc @noisysocks @richtabor
Hey @t-hamano 👋 Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note. We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1. All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :) Please let us know if you can assist with that. Thanks in advance :) |
@fabiankaegy Since this PR fixes unexpected behavior, I don't think a dev note is necessary. |
@t-hamano thanks for confirming :) will remove it from the list |
Related to #60447
What?
This PR changes the Advanced panel to not show to non-admin users.
Why?
The Advanced panel allows you to change attributes such as CSS classes, title, etc. This means updating the template that has that template part embedded in it, but non-admin users will get an error because they can't update the template:
How?
Add
canUserEdit
check.Testing Instructions