Fix block rename control shown in "Advanced" for unsupported blocks #55995
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
getdave
added
[Type] Bug
getdave
requested review from
kevin940726,
Mamaduka and
ellatrix
as code owners
|
Size Change: -5 B (0%) Total Size: 1.63 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
Nov 9, 2023
Comment on lines
149
to
169
| test( 'does not allow renaming of blocks that do not support the feature', async ( { | ||
| editor, | ||
| page, | ||
| pageUtils, | ||
| } ) => { | ||
| // Only Group supports renaming. | ||
| await editor.insertBlock( { | ||
| name: 'core/paragraph', | ||
| attributes: { content: 'First Paragraph' }, | ||
| } ); | ||
|
|
||
| // Multiselect via keyboard. | ||
| await pageUtils.pressKeys( 'primary+a' ); | ||
|
|
||
| // Expect the Rename control not to exist at all. | ||
| await expect( | ||
| page.getByRole( 'menuitem', { | ||
| name: 'Rename', | ||
| includeHidden: true, // the option is hidden behind modal | ||
| } ) | ||
| ).toBeHidden(); |
There was a problem hiding this comment.
Which part of the UI does this test? It's not clear from the test.
There was a problem hiding this comment.
The describe block provides the context. However the test itself could be made more focused by scoping the selectors to the relevant portion of the UI rather than the broad "rename" option.
There was a problem hiding this comment.
I'm assuming this tests menu item in the block toolbar options dropdown. Then, this expectation will never fail since the test doesn't open the dropdown, so the element === hidden assertion is always true.
That was the reason for my question because it wasn't clear where the "Rename" menu item should be located from the test.
Screenshot
There was a problem hiding this comment.
I've updated the scoping on the test. Feel free to merge if it looks good.
Mamaduka
approved these changes
Nov 9, 2023
scruffian
approved these changes
Nov 9, 2023
Nothing should happen to them. They stay as is. This gets merged directly into Then I have a separate task to effectively backport some stuff from here to merge it into Gutenberg |
|
I'm just working through the PRs that need cherry-picking for the next 6.4 point release and wanted to confirm that this will be included in the next package update in Core. |
mikachan
removed
the
Backport to WP Minor Release
|
@t-hamano Can I confirm that if you check out the |
|
Sorry for the late confirmation. Problem solved 👍
|
This was referenced Nov 20, 2023
3 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?
Ensures that block rename control is not shown for blocks that do not support the feature
Fixes #55953
Why?
Because only Group supports in 6.4. In
trunkthis is enabled for all blocks.How?
Gates the feature by the correct supports.
Testing Instructions
AdvancedpanelTesting Instructions for Keyboard
Screenshots or screencast