Fix template part actions in List View #48905
Merged
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
- Use the selectedClientIds param from the `BlockSettingsMenuControls` fill instead of selected blocks from the store
talldan
added
[Type] Bug
|
Size Change: -10 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
andrewserong
reviewed
Mar 8, 2023
There was a problem hiding this comment.
Great catch, and nice explanation of the issue!
It looks the same issue might also be happening with the Replace <BlockTitle /> (Replace Header) and Group menu items. In the below screenshot, the top-most group block's ellipsis menu is opened, but shows Replace Header because the Header block is selected:
Would it be good to try to fix them up at the same time, or in a separate PR, do you think?
Oh yeah, that's a shame. I'll do separate PRs! 😄 |
talldan
changed the title
|
I've made a PR to fix the grouping actions - #48910. The Replace template part actions are a little more complicated to untangle. |
andrewserong
approved these changes
Mar 8, 2023
There was a problem hiding this comment.
Thanks @talldan, good idea breaking it up into smaller PRs. This one looks good to go to me! ✨
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?
Fixes #48818
As described on the issue, the 'Create template part' or 'Convert to regular blocks' actions in the block settings menu don't always work as expected in List View.
For List View, a user can perform one of those actions on a non-selected block. The code was written with the assumption that the action would always be carried out on a selected block as would always be the case in the editor canvas.
I imagine that code predates the existence of the menu in List View.
How?
Updates the code to use the
selectedClientIdsparam from theBlockSettingsMenuControlsfill, which provides the correct block client Ids to perform the operation on.This required a little bit of a refactor. The
BlockSettingsMenuControlscomponent now has to be higher in the component tree. This doesn't seem to cause any unwanted side-effects from my testing.Testing Instructions
Template parts > My Group.In trunk: 🐞 Notice that the template part contains the Paragraph block, rather than the Group block.
In this PR: ✅ The template part correctly contains the group block
Screenshots or screencast
Before
Kapture.2023-03-08.at.11.43.27.mp4
After
Kapture.2023-03-08.at.11.39.50.mp4