Feature: Implement cursor navigation with navigation keys #404

juanca · 2020-07-18T20:19:23Z

Note: WIP! I am running into a few problems and would like advice.

What:

Implements navigation keys support in type method:

Arrows
Home and end
Page up and down

Why:

It is reasonable for a user to use keyboard navigation while interacting with an interactive element. An interactive element could be an input (where a user would type into) or an accessible UI with keyboard support (no typing but it handles key events).

How:

Handles new key events while typing: e.g. Brek{arrowleft}a{arrowright}fast -> Breakfast
Handles new key events with modifier while typing: e.g. Breakfast{shift}{arrowleft}{/shift} -> Breakfas{selection}t{/selection}

Checklist:

Documentation
Tests
Typings
Ready to be merged

Closes: #354

src/__tests__/type.js

juanca · 2020-07-18T20:22:37Z

src/keys/navigation-key.js

@@ -0,0 +1,58 @@
+import {fireEvent} from '@testing-library/dom'


What are your opinions on using this kind of code organization? I noticed there is 3 files for typing:

utils

type

type-modifiers

I think navigation keys will end up adding a chunk (but not massive) amount of code. I opted for a new file but I am willing to move things around.

I probably wouldn't have bothered making the new file myself, but I'm ok with it.

juanca · 2020-07-19T19:43:42Z

src/__tests__/type.js

+      "Bre{CURSOR}" -> "Brea{CURSOR}"
+    input[value="Brea"] - keyup: a (97)
+    input[value="Brea"] - keydown: ArrowLeft (37)
+    input[value="Brea"] - select


I am thinking a select event should show the before and after of the cursor position:

input[value="Brea"] - keydown: ArrowLeft (37) input[value="Brea"] - select "Brea{CURSOR}" -> "Bre{CURSOR}a" input[value="Brea"] - keyup: ArrowLeft (37)

What do y'all think?

For posterity's sake: there was an earlier comment and it got 1 response.

I'm not sure. Based on the playground we shouldn't fire a select event at all I think (though it may be unfortunately unavoidable).

I made a new comment thread to leave the unexpected {CURSOR} position bits out of this.

CWSites · 2020-10-15T18:57:22Z

Any update here? We could really use the ability to use type the arrow key for a couple reasons.

Test/verify accessibility issues
Get around complex components such as Material UI autocomplete by using our arrow key to select an option since we are unable to pick a specific option any other way.

DovRine · 2020-10-15T20:05:24Z

Agree with CWSites. This would greatly simplify testing things like: MaterialUI TextField[type="time"].

kentcdodds · 2020-10-15T20:09:06Z

I'm sure @juanca would be more than happy to have either of you pick up where they left off and address the merge conflicts and other issues in this PR :)

jesujcastillom · 2020-10-19T18:45:12Z

Hi! This is something I recently came across with, and I'd be happy to help if it needs picking up 😃

kentcdodds · 2020-10-19T23:35:18Z

Go for it 👍

juanca · 2020-10-19T23:40:13Z

@jesujcastillom Thanks for the offer! I would greatly appreciate the help. I've been nerd-sniped into other projects for my free time.

Let me know if anyone needs any context, happy to provide it whenever.

jesujcastillom · 2020-11-01T16:58:05Z

src/__tests__/type.js

+  expect(() => {
+    navigationKey('SOME_INVALID_KEY')
+  }).toThrowError(TypeError)
+})


I've modified the function to hit all the lines (default wasn't reachable) now the question is, should I include this test to make sure we throw a TypeError (and also throw it in the function)` if we don't get a valid key, or leave it without throwing, since it's an internal function anyway.

In summary, removing the last commit entirely, we still get 💯 coverage, but it's not really covered if an invalid key is passed, any thoughts? @juanca @kentcdodds

Note, modified the commit because I didn't have my git config setup properly in this machine, but yeah, same changes 😅

Is this even possible? We're the only ones who call navigationKey and we always call it properly. I'd just remove that logic.

No, it isn't I'll remove it :)

codecov · 2020-11-01T17:01:20Z

Codecov Report

Merging #404 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #404   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        13    +1     
  Lines          482       494   +12     
  Branches       134       136    +2     
=========================================
+ Hits           482       494   +12

Impacted Files	Coverage Δ
src/type.js	`100.00% <ø> (ø)`
src/keys/navigation-key.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee7c74f...522a362. Read the comment docs.

kentcdodds

Thanks for picking this up! Here's some feedback for you.

kentcdodds · 2020-11-02T21:14:38Z

src/type.js

@@ -44,6 +45,8 @@ const modifierCallbackMap = {
 }

 const specialCharCallbackMap = {
+  '{arrowleft}': navigationKey('ArrowLeft'),
+  '{arrowright}': navigationKey( 'ArrowRight'),


Suggested change

'{arrowright}': navigationKey( 'ArrowRight'),

'{arrowright}': navigationKey('ArrowRight'),

kentcdodds · 2020-11-02T21:24:59Z

src/__tests__/type.js

+  expect(getEventSnapshot()).toMatchInlineSnapshot(`
+    Events fired on: input[value="Brekafast"]
+
+    input[value=""] - pointerover


I just tried this in the playground and here's what I got for the events (Note, I ignored mouse/pointer move events because they're too noisy in the playground):

textarea#textarea[value=""] - pointerdown textarea#textarea[value=""] - mousedown textarea#textarea[value=""] - focusin textarea#textarea[value=""] - pointerup textarea#textarea[value=""] - mouseup textarea#textarea[value=""] - click textarea#textarea[value=""] - keydown textarea#textarea[value=""] - keypress textarea#textarea[value="b"] - input textarea#textarea[value="b"] - keyup textarea#textarea[value="b"] - keydown textarea#textarea[value="b"] - keypress textarea#textarea[value="br"] - input textarea#textarea[value="br"] - keyup textarea#textarea[value="br"] - keydown textarea#textarea[value="br"] - keypress textarea#textarea[value="bre"] - input textarea#textarea[value="bre"] - keyup textarea#textarea[value="bre"] - keydown textarea#textarea[value="bre"] - keypress textarea#textarea[value="brea"] - input textarea#textarea[value="brea"] - keyup textarea#textarea[value="brea"] - keydown // <- here's the arrow down to go left textarea#textarea[value="brea"] - keyup textarea#textarea[value="brea"] - keydown textarea#textarea[value="brea"] - keypress textarea#textarea[value="breka"] - input textarea#textarea[value="breka"] - keyup textarea#textarea[value="breka"] - keydown // <- here's the arrow down to go right textarea#textarea[value="breka"] - keyup textarea#textarea[value="breka"] - keydown textarea#textarea[value="breka"] - keypress textarea#textarea[value="brekaf"] - input textarea#textarea[value="brekaf"] - keyup textarea#textarea[value="brekaf"] - keydown textarea#textarea[value="brekaf"] - keypress textarea#textarea[value="brekafa"] - input textarea#textarea[value="brekafa"] - keyup textarea#textarea[value="brekafa"] - keydown textarea#textarea[value="brekafa"] - keypress textarea#textarea[value="brekafas"] - input textarea#textarea[value="brekafas"] - keyup textarea#textarea[value="brekafas"] - keydown textarea#textarea[value="brekafas"] - keypress textarea#textarea[value="brekafast"] - input textarea#textarea[value="brekafast"] - keyup

Notice that there's no "select" event in there. I'm not sure whether it's possible to avoid that event, but it would be good to avoid if we can.

Also, I think this test could be simplified if we change the input to: b{arrowleft}a{arrowright}c (which would result in abc, and much fewer events for the snapshot).

At least from what I could investigate, the [setSelectionRange](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#dom-textarea/input-setselectionrange] will fire a select event, and I can't really seem to find another way to move the cursor.

kentcdodds · 2020-11-02T21:26:16Z

src/__tests__/type.js

+  expect(() => {
+    navigationKey('SOME_INVALID_KEY')
+  }).toThrowError(TypeError)
+})


Is this even possible? We're the only ones who call navigationKey and we always call it properly. I'd just remove that logic.

kentcdodds

Looks good to me, but I don't think I can merge this until @juanca removes the draft status of this pr.

kentcdodds · 2020-11-03T17:35:40Z

@all-contributors please add @juanca for code and tests

@all-contributors please add @jesujcastillom for code and tests

allcontributors · 2020-11-03T17:35:45Z

@kentcdodds

I could not determine your intention.

Basic usage: @all-contributors please add @Someone for code, doc and infra

For other usages see the documentation

kentcdodds · 2020-11-03T17:36:02Z

@all-contributors please add @juanca for code and tests

allcontributors · 2020-11-03T17:36:10Z

@kentcdodds

I've put up a pull request to add @juanca! 🎉

kentcdodds · 2020-11-03T17:36:57Z

@all-contributors please add @jesujcastillom for code and tests

allcontributors · 2020-11-03T17:37:06Z

@kentcdodds

I've put up a pull request to add @jesujcastillom! 🎉

testing-library-bot · 2020-11-04T02:47:54Z

🎉 This PR is included in version 12.2.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

brandonpittman · 2020-11-04T02:56:06Z

Forgive my ignorance, but why only implement arrowright and arrowleft? Why not include up and down? My original interest in this issue was checking number inputs and the value associated when changing step by step. arrowleft and arrowright won't address that issue.

kentcdodds · 2020-11-04T03:25:52Z

Up and down would be extremely difficult 😬

brandonpittman · 2020-11-04T04:00:54Z

@kentcdodds What makes up and down harder than left and right?

kentcdodds · 2020-11-04T04:30:35Z

It comes down to where the cursor goes. In a textarea it depends on the size of the textarea as well as the font size. This would take a ridiculous amount of work (would probably require an environment that supports layout, so jsdom is out). Just too much work in afraid.

juanca commented Jul 18, 2020

View reviewed changes

src/__tests__/type.js Outdated Show resolved Hide resolved

juanca commented Jul 18, 2020

View reviewed changes

juanca commented Jul 19, 2020

View reviewed changes

jesujcastillom force-pushed the arrows-left-right branch from aab8cfa to e254f1c Compare October 27, 2020 11:03

jesujcastillom reviewed Nov 1, 2020

View reviewed changes

jesujcastillom force-pushed the arrows-left-right branch from 3c0fccc to 45445c3 Compare November 1, 2020 17:02

kentcdodds reviewed Nov 2, 2020

View reviewed changes

juanca and others added 4 commits November 3, 2020 17:26

feat(type): Implement cursor navigation with ArrowLeft and ArrowRight

07bbbef

refactor(navigation-key): use a Number instead to hit all the lines

94ca7b0

test(type): simplify snapshot

0faa576

style(type): remove whitespace

522a362

jesujcastillom force-pushed the arrows-left-right branch from 45445c3 to 522a362 Compare November 3, 2020 17:15

kentcdodds approved these changes Nov 3, 2020

View reviewed changes

kentcdodds marked this pull request as ready for review November 3, 2020 17:34

kentcdodds merged commit 01c3a56 into testing-library:master Nov 3, 2020

allcontributors bot mentioned this pull request Nov 3, 2020

docs: add juanca as a contributor #482

Merged

allcontributors bot mentioned this pull request Nov 3, 2020

docs: add jesujcastillom as a contributor #483

Merged

testing-library-bot added the released label Nov 4, 2020

sarahdayan mentioned this pull request Nov 10, 2020

docs: add {arrowleft} and {arrowright} special characters #486

Merged

4 tasks

nickmccurdy mentioned this pull request Nov 26, 2020

Add support for arrowdown and arrowup events #503

Closed

adamayres mentioned this pull request Mar 8, 2021

Arrowleft handler does not respect preventDefault resulting in incorrect selectionStart #578

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Implement cursor navigation with navigation keys #404

Feature: Implement cursor navigation with navigation keys #404

juanca commented Jul 18, 2020

juanca Jul 18, 2020

kentcdodds Jul 19, 2020

juanca Jul 19, 2020

juanca Jul 19, 2020

kentcdodds Jul 19, 2020

CWSites commented Oct 15, 2020

DovRine commented Oct 15, 2020

kentcdodds commented Oct 15, 2020

jesujcastillom commented Oct 19, 2020

kentcdodds commented Oct 19, 2020

juanca commented Oct 19, 2020

jesujcastillom Nov 1, 2020

jesujcastillom Nov 1, 2020

kentcdodds Nov 2, 2020

jesujcastillom Nov 3, 2020

codecov bot commented Nov 1, 2020 •

edited

kentcdodds left a comment

kentcdodds Nov 2, 2020

jesujcastillom Nov 3, 2020

kentcdodds Nov 2, 2020

jesujcastillom Nov 3, 2020

kentcdodds Nov 2, 2020

kentcdodds left a comment •

edited

kentcdodds commented Nov 3, 2020

allcontributors bot commented Nov 3, 2020

kentcdodds commented Nov 3, 2020

allcontributors bot commented Nov 3, 2020

kentcdodds commented Nov 3, 2020

allcontributors bot commented Nov 3, 2020

testing-library-bot commented Nov 4, 2020

brandonpittman commented Nov 4, 2020 •

edited

kentcdodds commented Nov 4, 2020

brandonpittman commented Nov 4, 2020

kentcdodds commented Nov 4, 2020

		@@ -0,0 +1,58 @@
		import {fireEvent} from '@testing-library/dom'

	'{arrowright}': navigationKey( 'ArrowRight'),
	'{arrowright}': navigationKey('ArrowRight'),

Feature: Implement cursor navigation with navigation keys #404

Feature: Implement cursor navigation with navigation keys #404

Conversation

juanca commented Jul 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CWSites commented Oct 15, 2020

DovRine commented Oct 15, 2020

kentcdodds commented Oct 15, 2020

jesujcastillom commented Oct 19, 2020

kentcdodds commented Oct 19, 2020

juanca commented Oct 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 1, 2020 • edited

Codecov Report

kentcdodds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kentcdodds left a comment • edited

Choose a reason for hiding this comment

kentcdodds commented Nov 3, 2020

allcontributors bot commented Nov 3, 2020

kentcdodds commented Nov 3, 2020

allcontributors bot commented Nov 3, 2020

kentcdodds commented Nov 3, 2020

allcontributors bot commented Nov 3, 2020

testing-library-bot commented Nov 4, 2020

brandonpittman commented Nov 4, 2020 • edited

kentcdodds commented Nov 4, 2020

brandonpittman commented Nov 4, 2020

kentcdodds commented Nov 4, 2020

codecov bot commented Nov 1, 2020 •

edited

kentcdodds left a comment •

edited

brandonpittman commented Nov 4, 2020 •

edited