-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prototype: move blocks between levels with keyboard #22453
Conversation
Size Change: +760 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
It's a really promising prototype. The improvements that make it possible to select nested blocks (#20732) in navigation mode work really well, and I think it'd be great to see them in a separate PR so that they can be shipped more quickly. And in terms of the Move To option, I've seen some similar systems for moving items around that work pretty well. Android TV uses a similar system for reordering apps, where an app is grabbed, moved and then dropped. There are a few things I found in testing (though I realise this is currently a prototype, so I understand they might already be part of the thought process):
|
Hi @tellthemachines 👋 This is really cool! The keyboard interaction feels natural. I too have similar feedback as @talldan's:
What I did was reference the block breadcrumbs at the bottom of the editor to be able to tell when the paragraph was outside the group. Overall, this is really promising! 👍 |
f83e557
to
d9e3c24
Compare
@talldan I've addressed all your comments except:
@enriquesanchez I have added an outline to the parent block, when focus is in an inner block, but when a root block is focused there isn't a good way to do it - it might be a bit weird to outline the whole document. What do you think? |
30b0036
to
b6c6865
Compare
Excited about this PR so I took the liberty of taking it for a spin. It's really impressive, nice work! The blue line works well, insofar as it appears to use the same style (perhaps even classname? 🤞 ) as should be visible in #21475. An indication for the "target nesting container" can work, but I agree the dotted isn't ideal. We probably shouldn't create a new treatment for this actually — can you try simply reusing the same style we use for indicating block focus? I.e. this one: The semantics of "focus" are a bit muddy here, since It's not actually what has focus. But it's in the same ballpark — if you press enter, that's the parent container it lands in. I think it can work. |
@tellthemachines Ah, that is a great point that I overlooked. I agree with you, only the parent should get the outline. We could also try doing something similar to #22508, to make it easier to identify the block structure. |
a04a737
to
d597ac0
Compare
Hmm, I'm not too keen on having extra return values as that will add unnecessary complexity to the code. Rather than that, I'll try to think of better naming for these functions. |
Good progress @tellthemachines One nit I picked is that in both Firefox and Chrome the editor does not scroll to where you are moving focus except after the moving mode is exited by pressing the Enter key: |
That is a navigation mode bug, reported in #22420. It should be fixed separately. I'll likely pick it up soon if no one does it in the meantime 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works as expected and offers a great feature for keyboard users. Well have some follow up PR take care of the navigation mode scrolling bug 👏
Description
Addresses #22031
Using the toolbar action described in the issue above to enable moving blocks in and out of nesting levels with keyboard only (something that is currently only possible with drag and drop).
I'm leveraging navigation mode for this, so I had to incidentally address #20732 too, to enable navigating in and out of nesting levels.Update: #20732 has been addressed separately.Setting this up as a draft so we can test the keyboard interaction. It doesn't yet work with a mouse/touchpad/touchscreen, as I'm not sure what the correct behaviour should be for those interaction modes.Update: this is now ready for review. Basic mouse interaction is enabled: clicking on "Move to" and then clicking once on the target block, then pressing Space or Enter, will move the block into place above the target block.All feedback and testing appreciated!
How has this been tested?
So far, tested only in browser with keyboard. A quick spin with VoiceOver shows the keyboard flow works, but there are no announcements of the actions taken (that should also be implemented once we decide if this is the way we want to go).
How it works:
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: