Allow arrow keys navigation in autocomplete list

[npny]

(Referencing element-hq/element-web#4737)

This allows left/right arrow keys to navigate backwards/forwards in the autocomplete list (on top of the up/down arrow keys which already did that).

The first commit is the minimal code change for that, while the rest are follow-up cleanup/refactor (now that names like onVerticalArrow, onUpArrow and onDownArrow make less sense).

Misc notes

• this.direction = '' needs to be at the top before the bunch of autocomplete.moveSelection, as the latter calls setState, which triggers onChange, which looks at this.direction, somewhere in the middle. It would be at the last keyDown event's value if it wasn't reset here first, which is incorrect.
• It arguably also should not be set by the current keyDown event, as this isn't supposed to be textual navigation, but instead is supposed to be "swallowed up" by the autocomplete list. This is why the rest of the // skip void nodes block is after the autocomplete.moveSelections, so that it only happens if the latter didn't match and didn't return true first.
• The switch condition can be collapsed further (see below), by relying on fall-through behaviour, but that was deemed unnecessarily confusing for future readers so I kept it as is.

switch (ev.keyCode) {
    case KeyCode.LEFT:
        this.autocomplete.moveSelection(-1);
    case KeyCode.RIGHT:
        this.autocomplete.moveSelection(+1);
    case KeyCode.UP:
        this.autocomplete.moveSelection(-1);
    case KeyCode.DOWN:
        this.autocomplete.moveSelection(+1);
        
    ev.preventDefault();
    return true;
}

• I took the liberty of removing the this.moveAutocompleteSelection -> this.autocomplete.onUpArrow indirection step as it didn't really add anything.

Signed-off-by: Pierre Boyer <pierre.s.boyer@gmail.com>

[bwindels]

Hi, thank you for your PR!

We've recently introduced a new composer for editing messages (currently under a labs flag, in src/components/views/elements/MessageEditor.js). The new editor shares the Autocomplete component with the MessageComposerInput. I'm still changing bits in the auto complete support there, so would rather wait until that stabilizes to see how these changes can be merged in.

Will report back here when that's done. Thanks for your patience.

[bwindels]

Apart from needing a few further changes, looking good already!

[bwindels]

Won't this prevent from ever going back selectionOffset=0 again? E.g. you'll just be able to cycle through pills without having the step with just the text you typed?

[npny]

You're right, good catch, I see the point behind the %(completionCount+1) now!

I've updated the above to:

// Note: selectionOffset 0 represents the unsubstituted text, while 1 means first pill selected
const index = (this.state.selectionOffset + delta + completionCount + 1) % (completionCount + 1);
this.setSelection(index);

[bwindels]

Thanks for the changes! Apart from left/right arrow comments, looks good! Still need to test it locally as well, but almost there :)

[bwindels]

No need to add left/right arrow keys for the new composer yet, I need to test if it doesn't interfere with caret navigation. Let's just do enough here so it isn't broken.

[bwindels]

No need to add left/right arrow keys for the new composer yet, I need to test if it doesn't interfere with caret navigation. Let's just do enough here so it isn't broken.

[npny]

Roger that, removed.

[bwindels]

Tried it locally, works well! Sorry it took so long. This looks good to merge now.