Allow navigation between blocks with arrow keys #1161
Conversation
|
Nice! 👍 I'd suggest adding the same behavior to Also, I noticed a "bug" in the button block where the focus moves to the link Editable. We could add tabindex="-1" maybe. Regardless, this is great 👍 |
blocks/editable/index.js
Outdated
| const moveDown = ( keyCode === DOWN || keyCode === RIGHT ) && this.isEndOfEditor(); | ||
| const selectors = [ | ||
| '*[contenteditable="true"]', | ||
| '*[tabindex]', |
There was a problem hiding this comment.
should we exclude tabindex=-1?
There was a problem hiding this comment.
Yeah, this won't account for all possibilities. Another being contenteditable without an explicitly true value (contenteditable, contenteditable=""). There's also select, button, object, area, a, though dunno if we want to test all of them in this behavior.
I'm forgetting the resource I used in creating it, but this implementation is close to spec for "focusable":
https://codepen.io/aduth/pen/wdRzxM
Maybe also worth moving out as a separate utility.
There was a problem hiding this comment.
Just found this library that looks interesting https://github.com/medialize/ally.js/blob/master/docs/api/README.md And it has a utility to find the tabbables elements https://github.com/medialize/ally.js/blob/master/docs/api/query/tabbable.md
There was a problem hiding this comment.
Wow, proper tabbable detection is complex!
https://github.com/medialize/ally.js/blob/master/src/is/tabbable.js
There was a problem hiding this comment.
Wow, yeah, that's a lot. I'm all for using a library for this if that's better.
|
@youknowriad Trying something slightly different at a block level... This approach looks at the range. If the range is the same, this means that nothing happened when pressing the arrow keys, which either means that the caret is at the start or end of text input, or that it's not text input. This seems to be good because it will handle more than just |
|
To do: use that library. |
|
This seems to work great for all our current blocks, but I wonder if it's not too aggressive. I mean, maybe a block with an input or something don't want to lose focus if we accidentally click on arrows. This question is I think hard to answer right now, so I'm ok with moving forward, I haven't seen any glitch (aside the link boundaries) |
|
Yeah, the links boundaries are an issue with either approach. |
|
Purely from a user's perspective, this is so much better. I'm undecided on how it handles the form fields... On one hand, it's not standard behaviour and yeah, we might lose focus accidentally. But on the other, it's great if I'm going from the bottom of the document to the top by holding down the 'up' arrow. I can completely navigate my post without leaving the arrow keys, and I only tab if I want to get into toolbars and other editing controls. However, if I was going up a document and ended up in an input field and arrow keys no longer took me up, that's expected behaviour and tabbing is the normal keyboard way of getting out of that, and would do what I expect. Nice! |
|
Working on redoing this with props right now. |
ellatrix
force-pushed
the
add/arrow-nav
branch
3 times, most recently
from
6b2caf1 to
255d12a
Compare
|
I think I'll leave table alone for now and do that in a separate PR. You can still navigate over it though. |
mtias
added
[Feature] Blocks
|
I like the flexibility of the current path because the block author has all the powers but I think I prefer the approach relying on "Tabbable" elements because it removes some burden from the block author. I hope I didn't push you to the wrong direction. Happy to follow the majority here. I wonder if there's not a middleground where we can do tabbables but we can deactivate it for more control and rely on props. Anyway, I think we should start with tabbables only. What do you think? |
|
Oh no, I really wish I saved the previous approach and didn't force push 🙈 Lesson learned |
|
@youknowriad So I really don't kind either approach, I can open a new PR with the previous approach. I'm not sure what the middle ground is you're suggesting. :) Maybe some more eyes could help here? @matias What do you think? |
|
Closes #1723. |
|
Follow up PR: #1778. |
|
Should this be closed as superseded by #1778 ? |
|
Yep |
This PR adds arrow key navigation between blocks in a very basic way, just like tabs would work. I took the approach of looking at focusable nodes instead of passing props, so block implementors don't have to worry about this as well.