Allow unselecting tags by keyboard

[qualitymanifest]

Fix

Closes #1852
If a tag is selected and the right arrow key is pressed, the tag is unselected and the focus is moved back into the tag field.

Test

See the issue for details to reproduce, but use right arrow to unselect

Review

Only one developer is required to review these changes, but anyone can perform the review.

Release

Release notes updated in bde11cf

[qualitymanifest]

Just realized I forgot about the tab key! I'll add that in in a second. I figure I'll leave the right arrow control in as well, that's the way that I would naturally think to exit the tag chip.

[dmsnell]

Hey @qualitymanifest - this is awesome work!

In looking at interceptKeys though it's clear that it was intended to prevent this very issue but for some reason that's not working. Although in testing it's clear that your patch fixes the issue at hand, I think the cause remains unresolved - merely worked-around.

In tag-input/index.tsx we can see that we only seem to call onChange during significant keyboard events vs. as the tag is being typed. Because of that we don't update this.state.tagInput in tag-field/index.tsx and our check '' !== this.state.tagInput never catches.

Do you think you might be interested in addressing the exchange between tag-field and tag-input? If we do that then we won't have to duplicate the logic.

We're happy to have your contributions so don't worry if you don't have time; I just want to make sure that we fix the problem where it originates.

[qualitymanifest]

It looks like tag-input's onChange does get called for regular keypresses though, inside onInput (a few times - might be worth looking into?), and this.state.tagInput does get updated. Here's a screenshot of two keypresses (red line separating them) with a bunch of console.logs peppered in:

I think the reason why tag-input doesn't intercept keyboard events when a tag is selected is because when a tag is "selected", the focus is actually on hidden-tag which is a child of tag-field and unrelated to tag-input

[qualitymanifest]

Actually now that I re-read your comment I'm confused regarding the interceptKeys comment, mainly because tag-field and tag-input both have an interceptKeys method so I'm not sure which one you're referring to.

tag-input's tab and and right-arrow handlers are for completing a suggestion, not for handling keyboard events when a tag-chip (really, hidden-tag) is "selected".

tag-field's only current key handler, backspace, is for deleting a tag-chip when it is "selected" - so I think it makes sense to also put the other tag-chip key handlers here.

I could definitely be missing something though!

[dmsnell]

@qualitymanifest what I was seeing at a cursory glance was that we only seem to be sending the updated tag text from tag-input to tag-field when we select a tag or accept the tag suggestion. because of this, when the tag-field intercepts the key presses it's making an assertion about the current tag-input text that's "out of date."

in other words, both components should agree that the input text is compu when we are typing computer but haven't yet finished typing. I believe that the parent component only sees `` until at the end of the typing it gets told "choose computer"

[qualitymanifest]

@dmsnell From my understanding, tag-field's state is getting updated on each keypress. tag-input's inputObserver is triggered on keypress and kicks off onInputMutation -> onInput -> onChange -> tag-field's storeTagInput where it is updated

That being said - it seems unrelated to the issue of being able to key out of a tag-chip. tag-field's current interceptKeys handler is only for selecting and deleting a tag chip, and there is no key handler to unselect that tag-chip

[dmsnell]

Thanks for your patience while I reviewed this @qualitymanifest. Everything you wrote seems sound and in testing worked just like you said.

There are some changes I'd like to apply to account for styling in the app and I couldn't remember how to update this PR directly so I was wondering if you'd be willing to apply this patch and push it out on the branch.

diff --git a/lib/tag-field/index.tsx b/lib/tag-field/index.tsx
index 2158790..55b9636 100644
--- a/lib/tag-field/index.tsx
+++ b/lib/tag-field/index.tsx
@@ -1,4 +1,4 @@
-import React, { Component } from 'react';
+import React, { Component, KeyboardEventHandler } from 'react';
 import { connect } from 'react-redux';
 import PropTypes from 'prop-types';
 import { Overlay } from 'react-overlays';
@@ -18,6 +18,10 @@ import {
   union,
 } from 'lodash';
 
+const KEY_BACKSPACE = 8;
+const KEY_TAB = 9;
+const KEY_RIGHT = 39;
+
 export class TagField extends Component {
   static displayName = 'TagField';
 
@@ -113,9 +117,8 @@ export class TagField extends Component {
 
   focusTagField = () => this.focusInput && this.focusInput();
 
-  interceptKeys = e => {
-    // handle backspace
-    if (8 === e.which) {
+  interceptKeys: KeyboardEventHandler<HTMLDivElement> = e => {
+    if (KEY_BACKSPACE === e.which) {
       if (this.hasSelection()) {
         this.deleteSelection();
       }
@@ -126,15 +129,18 @@ export class TagField extends Component {
 
       this.selectLastTag();
       e.preventDefault();
+      return;
     }
-    // handle right arrow
-    else if (39 === e.which && this.hasSelection()) {
+
+    if (KEY_RIGHT === e.which && this.hasSelection()) {
       this.unselect(e);
       this.focusTagField();
+      return;
     }
-    // handle tab
-    else if (9 === e.which && this.hasSelection()) {
+
+    if (KEY_TAB === e.which && this.hasSelection()) {
       this.unselect(e);
+      return;
     }
   };

[qualitymanifest]

Made the changes, reads a lot better with the constants @dmsnell