[Commands]: Fix move to command condition for registering
#54049
Conversation
This file contains hidden or 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
ntsekouras
added
[Type] Bug
|
Size Change: +16 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
Contributor
|
This does work but it's a bit confusing, including in the block menu. If I have a group block with a spacer block in it and under the group block there is an image block:
What was the original idea of limiting to only block? I can understand no having the move to available if there is only one block in the whole document. Otherwise I can't say what purpose does the limitation serve? Normally if I can drag and drop I should be able to move to. Move to is a keyboard accessible way to moving blocks. |
Mamaduka
reviewed
Aug 30, 2023
Contributor
Author
That's good thoughts but I don't have insights about the original idea and not great suggestions at the moment, as the Whatever logic we should choose though, we should be consistent in all places we have the |
Mamaduka
approved these changes
Aug 30, 2023
Member
Mamaduka
left a comment
Mamaduka
left a comment
There was a problem hiding this comment.
Let's land this ✅
@draganescu, great observations that we should address alongside other "Move to" issues. I believe @t-hamano created a few of those recently.
t-hamano
approved these changes
Aug 30, 2023
Contributor
t-hamano
left a comment
t-hamano
left a comment
There was a problem hiding this comment.
LGTM!
For now, I think it would be better to allow the command only if there are multiple blocks on the same level as well, to be consistent with the "Move to" menu on the block toolbar. As far as I know, this logic was introduced in #44827.
However, as @draganescu said, even if there is only one block on the same level, you can drag and drop it out of the parent block. Perhaps as a follow-up, to be more consistent with drag and drop, the "Move To" action should be allowed only when there is only one block in the entire document, both on the block toolbar and in the command.
Incidentally, the most serious problem we are experiencing in the Move to action is #54016, where the browser freezes when trying to move it into itself 😅
Contributor
Author
Oh... look at that.. I had introduced this logic 😆 |
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?
Follow up of: #53994, addressing @t-hamano 's comment for fixing the condition where the
move tocommand should be available.Testing Instructions
move tocommand should not be available when we have only one block at the first parent. Example is create a Group block and inserter just one block. Then test the command availability to that child block.