Page MenuHomePhabricator

ChipInput: improve keyboard navigation
Closed, ResolvedPublic

Description

Background

There are additional modes of keyboard navigation that would improve the ChipInput component:

  • In the input with no input value, backspace should focus the last FilterChip. (Backspacing again will remove the FilterChip - this is already implemented in the FilterChip component)
  • The arrow keys should allow you to navigate between the individual FilterChips and the input
  • The escape key should unfocus the input and create a new chip with the current value of the input (the latter part already works on blur)

User stories

As a user of the ChipInput component, I want to be able to smoothly navigate and use the input via the keyboard.

Previous implementations

  • OOUI: TagMultiselectWidget

Design spec

Open questions

  1. Is blur on esc needed?

Acceptance criteria (or Done)

Design

  • Update the component in the Figma library.

Code

  • Implement backspace behavior: when the input is empty, backspace focuses the last FilterChip
  • Implement arrow key + navigation: horizontal arrow keys navigate between individual FilterChips and the input
  • Implement escape key behavior: escape blurs the input and current input text becomes a new chip

Event Timeline

AnneT added a subscriber: Volker_E.

@Volker_E do we need to blur the input on escape? I don't believe this is default <input> behavior, and currently in the Codex version hitting escape just does nothing, no data is lost.

CCiufo-WMF triaged this task as Medium priority.Aug 28 2023, 3:35 PM
CCiufo-WMF renamed this task from FilterChipInput: improve keyboard navigation to ChipInput: improve keyboard navigation.Aug 30 2023, 6:54 PM

Additionally, I think that when a chip is deleted using the keyboard, the focus should not go back to the input, but should go to the next chip (if delete was pressed) or the previous chip (if backspace was pressed). If there are no chips left, or if the delete key was pressed on the last chip in the list, then the input should be focused.

Change 955852 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] ChipInput: Add keyboard navigation

https://gerrit.wikimedia.org/r/955852

@bmartinezcalvo The patch for this is currently in code review, but it's also ready for an initial design review if you have time. You can test it here.

I implemented the following keyboard interactions:

When the input is focused:

  • Enter: add a chip with the entered text (we already had this)
  • Escape: blur (remove focus); if there is text in the input, that text becomes a new chip
  • Backspace: if the cursor is at the start of the input, move the focus to the last chip; if the cursor is not at the start of the input, allow the normal behavior to happen (deleting a character)
  • Left arrow (LTR) / right arrow (RTL): same as backspace

When a chip is focused:

  • Enter: edit the chip (we already had this)
  • Backspace/Delete: delete the chip (we already had this); move focus to the previous chip (for Backspace) / next chip (for Delete)
  • Left/right arrow: move focus to the previous/next chip (in LTR, left=previous and right=next, in RTL it's the other way around)
  • Escape: blur (remove focus)

Open question:

  • If there is text in the input, and you use the arrow keys to move through the text and then to one of the chips, what should happen?
    • Option 1: the text becomes a new chip. This is what happens in my patch, because the code creates a new chip when the input loses focus. When you move to a chip with the arrow keys, the focus moves from the input to that chip, and that counts as the input losing focus. (If the text is a duplicate of an existing chip, it stays in the input and a duplicate chip is not created.)
    • Option 2: the text stays in the input. We could change the code to create a new chip only when the focus leaves the component as a whole, but not when it moves between elements within the component. Then arrow-keying onto a chip would leave the text alone, but moving the focus to somewhere else outside the ChipInput component would turn the text into a chip.

@bmartinezcalvo The patch for this is currently in code review, but it's also ready for an initial design review if you have time. You can test it here.

I implemented the following keyboard interactions:

When the input is focused:

  • Enter: add a chip with the entered text (we already had this)
  • Escape: blur (remove focus); if there is text in the input, that text becomes a new chip
  • Backspace: if the cursor is at the start of the input, move the focus to the last chip; if the cursor is not at the start of the input, allow the normal behavior to happen (deleting a character)
  • Left arrow (LTR) / right arrow (RTL): same as backspace

When a chip is focused:

  • Enter: edit the chip (we already had this)
  • Backspace/Delete: delete the chip (we already had this); move focus to the previous chip (for Backspace) / next chip (for Delete)
  • Left/right arrow: move focus to the previous/next chip (in LTR, left=previous and right=next, in RTL it's the other way around)
  • Escape: blur (remove focus)

@Catrope thank you, I've reviewed all these new cases and they work well in the patch. Also added them in the keyboard navigation table in the Figma spec sheet.

Open question:

  • If there is text in the input, and you use the arrow keys to move through the text and then to one of the chips, what should happen?
    • Option 1: the text becomes a new chip. This is what happens in my patch, because the code creates a new chip when the input loses focus. When you move to a chip with the arrow keys, the focus moves from the input to that chip, and that counts as the input losing focus. (If the text is a duplicate of an existing chip, it stays in the input and a duplicate chip is not created.)
    • Option 2: the text stays in the input. We could change the code to create a new chip only when the focus leaves the component as a whole, but not when it moves between elements within the component. Then arrow-keying onto a chip would leave the text alone, but moving the focus to somewhere else outside the ChipInput component would turn the text into a chip.

@Catrope option 2 looks more reasonable to me. I would create the chip just when using enter, esc or clicking outside the input.

Change 955852 merged by jenkins-bot:

[design/codex@main] ChipInput: Add keyboard navigation

https://gerrit.wikimedia.org/r/955852

Change 956512 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] ChipInput: Add input text as chip when focus leaves component, not input

https://gerrit.wikimedia.org/r/956512

Additionally, I think that when a chip is deleted using the keyboard, the focus should not go back to the input, but should go to the next chip (if delete was pressed) or the previous chip (if backspace was pressed). If there are no chips left, or if the delete key was pressed on the last chip in the list, then the input should be focused.

I think that's fine, although I was a bit concerned about mistakenly double-pressing backspace at first. As soon as the focus is on one of the chips and you double press backspace, you might loose a FilterChip unintendedly. Alternative would be backspace delete, input focus, backspace focus next end chip, backspace delete. Specifically users with motor/sensoric disabilities might too quickly remove several FilterChips.
The alternative sadly can't make up for navigating with arrow keys and then deleting in the middle of a FilterChip sequence, so the current solution might be fine enough for now (until we hear certain user feedback).

@bmartinezcalvo The patch for this is currently in code review, but it's also ready for an initial design review if you have time. You can test it here.

I implemented the following keyboard interactions:

When the input is focused:

  • Enter: add a chip with the entered text (we already had this)
  • Escape: blur (remove focus); if there is text in the input, that text becomes a new chip
  • Backspace: if the cursor is at the start of the input, move the focus to the last chip; if the cursor is not at the start of the input, allow the normal behavior to happen (deleting a character)
  • Left arrow (LTR) / right arrow (RTL): same as backspace

When a chip is focused:

  • Enter: edit the chip (we already had this)
  • Backspace/Delete: delete the chip (we already had this); move focus to the previous chip (for Backspace) / next chip (for Delete)
  • Left/right arrow: move focus to the previous/next chip (in LTR, left=previous and right=next, in RTL it's the other way around)
  • Escape: blur (remove focus)

Open question:

  • If there is text in the input, and you use the arrow keys to move through the text and then to one of the chips, what should happen?
    • Option 1: the text becomes a new chip. This is what happens in my patch, because the code creates a new chip when the input loses focus. When you move to a chip with the arrow keys, the focus moves from the input to that chip, and that counts as the input losing focus. (If the text is a duplicate of an existing chip, it stays in the input and a duplicate chip is not created.)
    • Option 2: the text stays in the input. We could change the code to create a new chip only when the focus leaves the component as a whole, but not when it moves between elements within the component. Then arrow-keying onto a chip would leave the text alone, but moving the focus to somewhere else outside the ChipInput component would turn the text into a chip.

I'd like to bring up a third proposal that is contradicting some of the work already established, but for a considerate reason IMHO:
Escape never stands for entering information, so it feels contradicting to me to have blur turn the input value into a tag. I think just Enter should do so and Escape should only remain the value in the input until the user turns it into a FilterChip.

Change 956973 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from v0.18.0 to v0.19.0

https://gerrit.wikimedia.org/r/956973

Change 956973 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.18.0 to v0.19.0

https://gerrit.wikimedia.org/r/956973

If a user enters some text ("foo") and then hits the tab key, that will create chip "foo" inside the input and focus will be moved outside of the input entirely (forcing the user to click or tab backwards if they want to add multiple chips).

Is this behavior correct? In my mind, the expected behavior is:

  1. User enters text "foo"
  2. User hits tab key
  3. A "Foo" chip gets created and focus is returned to the text input (now empty except for the existing chip)
  4. User hits tab key again (while no text is present in input)
  5. Focus is moved out of the component to next focusable element

This would make it easier for users to add multiple chips using only the keyboard.

If you press "Enter" instead of "Tab", thats get you the behavior that you want, right? I think the Tab behavior is expected, because Tab moves the focus forward and that's what it does in your example.

Change 956512 merged by jenkins-bot:

[design/codex@main] ChipInput: Add input text as chip when focus leaves component, not input

https://gerrit.wikimedia.org/r/956512

I'd like to bring up a third proposal that is contradicting some of the work already established, but for a considerate reason IMHO:
Escape never stands for entering information, so it feels contradicting to me to have blur turn the input value into a tag. I think just Enter should do so and Escape should only remain the value in the input until the user turns it into a FilterChip.

@Volker_E although esc is usually used to close things instead of create them, in this case pressing esc is a way to create a new chip as we create a new chip when typing in the input and clicking outside it (finishing typing). So I agree with using esc to create a new chip in this case.

Design sign-off done. The new keyboard behaviors look good to me:

  • The text typed within the input becomes a new chip when pressing either tab , enter , or escape .
  • backspace: If the focus is placed on a chip, this key removes the chip and moves the focus to the previous chip. If the cursor is at the start of the input, move the focus to the last chip.
  • + navigation: Arrow keys navigate between the chips within the input. The behavior is correctly mirrored for RTL view.
  • (escape) behavior: If the user is typing in the input, this key creates a chip with that text typed. This key also removes the focus from the focused element.
    • Option 2 implemented correctly: These keys move the cursor through the letters when typing in the input. Once they jump to the chip, the typed text remains text.

If no further implementation/updates are required, we can move this task to Pending Release.

Change 961452 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update Codex from v0.19.0 to v0.20.0

https://gerrit.wikimedia.org/r/961452