-
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
Render non-editable preview of template part when user does not have capability to edit template part #60326
Render non-editable preview of template part when user does not have capability to edit template part #60326
Conversation
…rights to edit template part
Size Change: +282 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
} ) { | ||
useBlockEditingMode( 'disabled' ); | ||
|
||
const { content, editedBlocks } = useSelect( |
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.
What's the difference between all the code that is here and the useEntityBlockEditor
hook?
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.
Actually I missed something here. The difference should be that the getEntityRecord
call here uses context: 'view'
which the useEntityBlockEditor
hook does not support
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.
Can we add a "readonly" option to the useEntityBlockEditor
hook to address this?
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.
@youknowriad updated this in 352aa9e. This removes a lot of the complexity because after testing a bit more it seems we can pretty much omit edits for user roles that cannot edit the template.
This also fixes the request to use the view context as I mentioned
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.
That's better but I still wonder whether we should just reuse useEntityBlockEditor with an option? Wdyt?
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.
Hmm yeah, I hear your concerns. I'll have to see whether I can update the useEntityBlockEditor
hook without adding too much complexity. It's not obvious to me how to best approach it right now but will experiment a bit.
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.
Okay I'm having a hard time modifying the useEntityBlockEditor
hook. We can't call any of the hooks there conditionally but want to avoid doing a bunch of the work in case of the readonly mode.
@youknowriad you let me know how you want to handle this. Do you want to push something to this branch? Do you want me to merge as is and then simplify it later? Open to whatever you think makes the most sense
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.
Let's merge as is. It doesn't introduce any API and address a need. We can consider the refactoring later if needed.
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. |
tagName: TagName, | ||
blockProps, | ||
} ) { | ||
useBlockEditingMode( 'disabled' ); |
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.
Can we replace this with templateLock: 'all' ?
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 tried that first. But template lock all only prevents moving / deleting / inserting blocks. Not changing the attributes of any of the blocks that are already in there.
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 wish we could make another "lock" level rather than a hook like that but that's fine for now (declarative means less bugs in general)
…iew context there are no edits
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 think we can probably avoid two components and have a single one with a readonly mode based on the permissions check but This is working anyway. and that's just implementation details.
…capability to edit template part (WordPress#60326) Co-authored-by: fabiankaegy <[email protected]> Co-authored-by: youknowriad <[email protected]>
return { | ||
canViewTemplatePart: | ||
select( coreStore ).canUser( 'read', 'templates' ) ?? false, | ||
canEditTemplatePart: | ||
select( coreStore ).canUser( 'create', 'templates' ) ?? | ||
false, | ||
}; |
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.
@fabiankaegy, I've got a similar question here. Should this check if a user can read/edit this particular template part, or is it just a general check for templates?
What?
Part of #60316. Building on top of: #60317 and #58301
Adds a new non-editable mode to the template part block that renders the block list as disabled and non-editable.
Why?
This mode will be needed to render a preview of a template part for users that can view a template but not edit it.
How?
Adding a user capability check before rendering the editable template part preview and for users that cannot edit it rendering an alternative locked down mode.
Testing Instructions
Apply the following patch locally:
Open the site editor and see that the template parts like the header are rendering fully but are not editable/selectable.