-
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
Editor: Reuse the tabbable utility to retrieve the tabbables elements in WritingFlow #2696
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2696 +/- ##
==========================================
+ Coverage 33.64% 33.65% +<.01%
==========================================
Files 185 185
Lines 5584 5583 -1
Branches 973 972 -1
==========================================
Hits 1879 1879
Misses 3138 3138
+ Partials 567 566 -1
Continue to review full report at Codecov.
|
editor/writing-flow/index.js
Outdated
.filter( ( node ) => ( | ||
node.nodeName === 'INPUT' || | ||
node.nodeName === 'TEXTAREA' || | ||
( node.isContentEditable && node.contentEditable !== 'inherit' ) || |
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.
Could this be simplified to node.contentEditable === 'true'
?
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.
sure
e6b9103
to
4fe0ede
Compare
4fe0ede
to
5ea104c
Compare
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.
Discussed 1-to-1: This seems an improvement, but it's not clear to me the specific behavior we want for arrow navigations, since in some but not all cases we want the block to receive focus (i.e. when there aren't other input fields within). Or at least this is how it appears from the user's perspective. In reality, the block receives focus but then the editable takes over and steals focus to it. I think in a subsequent pull request we might want to consider consolidating this (from Editable to WritingFlow?) and being more explicit about where arrow keys should land focus for the user.
this reuses the tabbable utility to ensure a more robust arrow navigation.
Testing instructions
I'm finding that the button block works better now, it used to be a caret trap but seems to work now.