List view: show context menu for content-only blocks in posts #62354
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
ramonjd
added
Needs Design
|
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. |
|
Thanks for the quick work here! I know this is a WIP so you've probably already seen, but just in case, it looks like the 
|
|
Size Change: +78 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in e9da438. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9412529161
|
ramonjd
commented
Jun 6, 2024
There was a problem hiding this comment.
Pinging @kevin940726 to check the change to the templateId condition.
Thanks!
| [ clientId ] | ||
| ); | ||
|
|
||
| if ( ! canEditTemplateParts ) { |
There was a problem hiding this comment.
I also looked into just showing a lock icon like Gutenberg did before.
I can't do a canUser check in the list item component as it's from the core store and can't be imported into the block editor package.
Open to suggestions!
packages/editor/src/components/block-settings-menu/content-only-settings-menu.js
Outdated
Show resolved
Hide resolved
| return ( | ||
| <> | ||
| <BlockSettingsMenuFirstItem> | ||
| <MenuItem |
There was a problem hiding this comment.
So this was hacked together and needs review.
There are two items — MenuItem and Text — because there is a line separating the two that persists.
Ideally, the dropdown wouldn't be required at all and we'd replace the ellipsis menu with a lock icon.
Ran out of time to investigate that today.
There was a problem hiding this comment.
I don't think the lock icon is desired, as users can still edit the content of blocks like Post Title, even with low permissions. So this is on the right track, but the copy needs to be adjusted.
I think some text like "Only users with permissions to edit the template can move or delete this block" is more in the spirit of #61127. Perhaps with an aria-disabled 'Edit Template' button.
@jameskoster may have some suggestions.
TLDR for you - the question is around what to show in list view here when the user doesn't have permissions to edit the template:
There was a problem hiding this comment.
I don't think the lock icon is desired, as users can still edit the content of blocks like Post Title, even with low permissions. So this is on the right track, but the copy needs to be adjusted.
I think some text like "Only users with permissions to edit the template can move or delete this block" is more in the spirit of #61127.
I like this. I've updated the PR so that we're using the existing component, but disabling the button and adding the suggested text for now, until further design guidance 😄
Yep, thanks @andrewserong I saw that and was struggling with how to best satisfy all the conditions, so I just committed what I had locally in 4dd5b63 and am hoping that the rest comes out in testing. 😄 I'll carry on with this tomorrow. |
talldan
reviewed
Jun 6, 2024
packages/editor/src/components/block-settings-menu/content-only-settings-menu.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/block-settings-menu/content-only-settings-menu.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/block-settings-menu/content-only-settings-menu.js
Outdated
Show resolved
Hide resolved
ramonjd
commented
Jun 6, 2024
packages/editor/src/components/block-settings-menu/content-only-settings-menu.js
Outdated
Show resolved
Hide resolved
|
On trunk, it shows
But in this PR it shows
(By contentOnly blocks, I mean the one inside <!-- wp:group {"templateLock":"contentOnly","layout":{"type":"constrained"}} -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>content locked</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->I think this is why I added the |
I wonder that too! If you have any ideas... otherwise I can take a brief look today, but very happy if you wanna push a commit here too 😄 |
Except when it's a page 😄 
I guess there should be some condition that also takes into account the post editor. This makes me think the But I'm not sure what the right condition would entail. Maybe we need to resort to checking the block name? |
I've done that in c50b7ba to test it out |
ramonjd
force-pushed
the
try/list-view-edit-template
branch
from
c50b7ba to
e9da438
Compare
|
Should this be backported for WP 6.6? If so, please add the |
👍🏻 Not sure if we've found the right solution yet, but I'll add it optimistically. Thanks! |
ramonjd
added
the
Backport to WP 6.7 Beta/RC
I haven't looked too deeply at the problem, but there might be some inspiration in the block inspector and the way it uses the |
Oh terrific, thanks @talldan I'll run a test on it. |
…e editor pages by checking only for a templateId. Previously it was only shown for pages and there was no check if the user can edit template. Show a not-very-pretty dialogue box where a user cannot edit a template.
Move canUser fallback component beneath existing entity check block
ramonjd
force-pushed
the
try/list-view-edit-template
branch
from
e9da438 to
e17d37d
Compare
Whoops, too many exclamation marks :)
ramonjd
force-pushed
the
try/list-view-edit-template
branch
from
9218b6e to
77a02a4
Compare
This appears to work as expected, and actually corresponds with the selector in the TemplateLockContentOnlyMenuItems component. 👍🏻 The selector checks the ancestor blocks to see if they're a pattern gutenberg/packages/block-editor/src/store/private-selectors.js Lines 474 to 486 in 9218b6e The only thing I'm not certain about is what should happen after a user has unlocked the block. The message says:
Is it supposed to be temporary? I'm testing in the post editor, and once I unlock it, it stays that way 😄 Example locked content<!-- wp:group {"templateLock":"contentOnly","layout":{"type":"constrained"}} -->
<div class="wp-block-group"><!-- wp:image {"id":27,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image size-full"><img src="http://localhost:8888/wp-content/uploads/2024/06/white-bellied-swallow-rotated.jpg" alt="" class="wp-image-27"/></figure>
<!-- /wp:image -->
<!-- wp:cover {"overlayColor":"accent","isUserOverlayColor":true,"isDark":false,"layout":{"type":"constrained"}} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="wp-block-cover__background has-accent-background-color has-background-dim-100 has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">content locked cover</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->
<!-- wp:paragraph -->
<p>content locked</p>
<!-- /wp:paragraph -->
<!-- wp:group {"layout":{"type":"constrained"}} -->
<div class="wp-block-group"><!-- wp:paragraph -->
<p>content locked nested</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group --> |
It seems to be working as expected on my end. It should select the parent block and show the "Done" button on the toolbar. It's not very noticeable, though. There's a separate issue #61554 tracking that. Is this what you mean here? |
Thanks for checking @kevin940726 - yep that's exactly what I meant. So I just didn't see the "Done" button 🙈 Ignore me then. |
kevin940726
approved these changes
Jun 11, 2024
ellatrix
added a commit
that referenced
this pull request
Jun 11, 2024
* Ensure the Edit template context menu is shown in the post editor/site editor pages by checking only for a templateId. Previously it was only shown for pages and there was no check if the user can edit template. Show a not-very-pretty dialogue box where a user cannot edit a template. * Check for an entity before showing the canUser message. * Rename variable Move canUser fallback component beneath existing entity check block * Use existing component but disable the edit button and update the copy * Check for content blocks * Use `getContentLockingParent` Co-authored-by: ramonjd <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: kevin940726 <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ellatrix <[email protected]>
ellatrix
added a commit
that referenced
this pull request
Jun 11, 2024
* Ensure the Edit template context menu is shown in the post editor/site editor pages by checking only for a templateId. Previously it was only shown for pages and there was no check if the user can edit template. Show a not-very-pretty dialogue box where a user cannot edit a template. * Check for an entity before showing the canUser message. * Rename variable Move canUser fallback component beneath existing entity check block * Use existing component but disable the edit button and update the copy * Check for content blocks * Use `getContentLockingParent` Co-authored-by: ramonjd <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: kevin940726 <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ellatrix <[email protected]>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
…ess#62354) * Ensure the Edit template context menu is shown in the post editor/site editor pages by checking only for a templateId. Previously it was only shown for pages and there was no check if the user can edit template. Show a not-very-pretty dialogue box where a user cannot edit a template. * Check for an entity before showing the canUser message. * Rename variable Move canUser fallback component beneath existing entity check block * Use existing component but disable the edit button and update the copy * Check for content blocks * Use `getContentLockingParent` Co-authored-by: ramonjd <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: kevin940726 <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: ellatrix <[email protected]>
ellatrix
removed
the
Backport to WP 6.7 Beta/RC
|
This was cherry-picked to the wp/6.6 branch. |
2 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.
What?
Maybe resolves #62352
Ensures the Edit template context menu is shown for all post types in the post editor/site editor pages.
Display a not-very-pretty dialogue box where a user cannot edit a template.
Why?
Since #61127, a context menu is displayed for list view items that a locked, e.g., template parts and blocks
Previously the context menu was only shown for pages, and there was no check if the user can edit template.
How?
Rather than check for a post type, just look for a
templateId.A
templateIdwill exists for post types that have a template and not for templates, patterns, template parts themselves.Testing Instructions
As admin, open up a post in the post editor. In the post settings, click on "Show template"
Open the list view and click on the ellipsis menu for template-related items, e.g., Title, Content and any template parts like a Header.
2024-06-07.16.15.57.mp4
Ensure that you can see the edit context menu above.
Template parts and other page/post template blocks such as post title, content and featured images should have the "Edit template" context menu.
Locked items in the page/post content will get the "Unlock" context menu.
Check that all the above applies for pages in the post editor and the site editor.
Now log in as an Author, create a new post. In the post settings, click on "Show template".
Open the list view and click on the ellipsis menu for template-related items.
You should see a message that explains that you can't edit the block.