Remove lock icons from Content blocks inner blocks when editing a page in the site editor #61922
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
talldan
added
[Type] Regression
|
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: -9 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
|
Seems this breaks the "Zoom Out › Clicking on inserter while on zoom-out should open the patterns tab on the inserter" test, which is strange. Will look into it. |
This test fails on I think the test is very flaky (#61806) 🤷 |
|
Before #61127 top level blocks were considered locked (which they are) in template-locked mode but not in this PR. |
@youknowriad Yes, that's by design, there was a design decision to remove the locked icon for content only locked blocks. |
|
This fix is working for me 👍 |
youknowriad
approved these changes
May 24, 2024
|
Actually, in template-lock mode, these top level blocks are entirely locked and not just "content only". So I think they should have the lock icon. |
I might be misunderstanding what you mean by 'template-locked' mode. I'm testing by editing a page. For me I can still change the title and featured image content. The Header/Footer I can't, but they have an 'Edit' button. |
|
Ok, so I did more tests, it seems that "non post blocks" like random paragraphs present in the template are hidden entirely from the list view, rather than kept with the lock icon. Isn't that a bit confusing? |
|
Maybe it's out of scope of this PR though, so I'm fine if this is treated separately but I wonder if this was discussed and something that was agreed upon. Don't want to block this PR. |
|
Yeah, I think it was a design decision to try to simplify the hierarchy of blocks. If all the various layout blocks that make up a template are included it becomes quite complicated. I think the idea started from contentOnly mode, and it's been that way for page editing going back to the original work that @noisysocks was involved in when Synced patterns now also work the same way, hiding the completely locked blocks and only showing what's editable in List View. |
|
The failing test is now skipped in trunk - #61925. So I'll merge this even though it's failing. |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
… to contentOnly (WordPress#61922) Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
… to contentOnly (WordPress#61922) Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@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?
It was mentioned in #61127 (comment) that there was a regression around how lock icons are shown when editing a page in the site editor.
Previously, blocks within the 'Content' block were shown without the lock icon as they can be freely edited, but after #61127 they show a lock icon.
Why?
The regression seems to have happened because of a change to the
canMoveBlockselector, which is called as part of the logic for displaying the lock icon. The change considered any block with a parent that hasblockEditingMode === 'contentOnly'as not moveable, but that's not the case.My understanding is there are some differences between
blockEditingMode === 'contentOnly'andtemplateLock === 'contentOnly'that can be hard to understand.blockEditingModedoesn't affect inner blocks, whiletemplateLockdoes.The selector is probably missing some logic for
templateLock === 'contentOnly', but I'd consider that for a different PR.How?
Revert the changes to the selector. This may bring back some other issues that the change was intended to solve.
Testing Instructions
In addition to the steps below, it's also worth smoke testing other editing modes (editing template, template parts, posts etc.)
Page editing
Expected: The lock icons shouldn't be displayed on the inner blocks of the 'Content' block. The templated blocks (Title, Featured Image, Content) are also not expected to show a lock icon since #61127, the block options dropdown menu explains why those blocks can't be moved.
Synced patterns with overrides
Expected: The lock icons are not displayed on overriden block with a synced pattern. The block options dropdown menu explains why those blocks can't be moved.
contentOnly locked groups
"templateLock":"contentOnly"attribute to the group blockExpected: The lock icons are not displayed on the inner blocks of the group. The block options dropdown menu explains why those blocks can't be moved.
Screenshots or screencast
Page editing in the site editor
Synced patterns with overrides
Groups with
templateLock === 'contentOnly'