Skip to content
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

[Combobox] ctrl+a in text input replaces selection when multiple=true #499

Open
mr-tech opened this issue Oct 12, 2024 · 1 comment
Open

Comments

@mr-tech
Copy link

mr-tech commented Oct 12, 2024

Describe the bug
Pressing ctrl + a replaces current selected values with that of all the visible items in the dropdown, even when text input is focused.

To Reproduce
Steps to reproduce the behavior:

https://stackblitz.com/edit/github-1j1ryx-1vucsx?file=src%2Froutes%2Findex.tsx

<Combobox<string>
  options={options.latest ?? []}
  placeholder="Search a fruit…"
  itemComponent={(props) => (
    <Combobox.Item item={props.item} class="item">
      <Combobox.ItemIndicator></Combobox.ItemIndicator>
      <Combobox.ItemLabel>{props.item.textValue}</Combobox.ItemLabel>
    </Combobox.Item>
  )}
  multiple
>
  <Combobox.Control class="control">
    <Combobox.Input class="input" />
    <Combobox.Trigger>
      <Combobox.Icon />
    </Combobox.Trigger>
  </Combobox.Control>
  <Combobox.Portal>
    <Combobox.Content class="content">
      <Combobox.Listbox />
    </Combobox.Content>
  </Combobox.Portal>
</Combobox>

(There are extra properties in the stackblitz that are not necessary to reproduce this behavior. It occurs in the docs examples as well.)

  1. Create a Combobox with the multiple flag (true)
  2. Open the dropdown list so that there are items in it (either by typing a valid string, or clicking the trigger)
  3. Press ctrl+a
  4. All items are now selected as if each one had been individually selected from the list

Expected behavior
When the text input field is focused, ctrl + a should only select (highlight) the text in the field. I can't think of any other behavior that should happen instead of, or in addition to this.

Screenshots

chrome_5zPbe3hYTb.mp4

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome
  • Version: 0.13.6 (kobalte version)

Additional context
ctrl + a is a common way to highlight all text in a focused input field for quickly deleting or replacing it. The Combobox input field behaves as this way (as expected) when the Comboxbox is not set to multiple. When multiple is set, the behavior changes to that explained above. There doesn't seem to be any way for a user to change this behavior.

I'm not sure what I would expect if focus was instead inside of the dropdown list. In that case, I could maybe see the current behavior making sense (ctrl + a replacing the selected values with that of the visible items).

Thumbing around a bit, I think it's related to

const selectableCollection = createSelectableCollection(
more specifically
case "a":
if (
isCtrlKeyPressed(e) &&
manager.selectionMode() === "multiple" &&
access(mergedProps.disallowSelectAll) !== true
) {
e.preventDefault();
manager.selectAll();
}
break;

@mr-tech
Copy link
Author

mr-tech commented Oct 17, 2024

I'll note that at first glance, it looks like either forcing disallowSelectAll: true in the createSelectableCollection() call, or surfacing the prop may be a the quickest solutions requiring only one line of code and few lines respectively.

Also, doing a quick search of the code base and docs, no disallowSelectAll is ever surfaced as an option to set on components that use a Selectable Collection. It seems only when directly calling createSelectableCollection can that value be set by a user. Considering the docs don't make mention of this function, and nowhere else in the codebase does this disallowSelectAll ever even get referenced, I'm not entirely sure the usefulness of this check as a property.

To be clear, I think that this kind of behavior should opt-in by default if it's to exist at all, but considering that may be considered a breaking change, I'm fine enough with just being able to turn it off manually.

Edit: Looking at things more closely, I missed this comment in the code:

// Use `createSelectableCollection` to get the keyboard handlers to apply to the input.
const selectableCollection = createSelectableCollection(
{
selectionManager: () => listState.selectionManager(),
keyboardDelegate: delegate,
disallowTypeAhead: true,
disallowEmptySelection: true,
shouldFocusWrap: () => local.shouldFocusWrap,
// Prevent item scroll behavior from being applied here, handled in the Listbox component.
isVirtualized: true,
},
inputRef,
);

"And while the developer can pass in their own KeyboardDelegate, it seems the purpose of that is only for navigation (though one could override the behavior to select as well I'm sure). The ctrl + a, Tab and Escape interactions are inaccessible from the Combobox component even through the KeyboardDelegate—"

Is what I would say but the other defined behaviors of including the Arrow keys, Home, End, and Page Up / Down don't do much of anything when the input has focus. And as far as I can tell there's no way to actually get focus inside the dropdown list; Tab exits the input which clears it and closes the dropdown. And as noted in another issue, the input is not controllable so this will always happen. Not being able to move focus into the dropdown is another bug considering it destroys accessibility, but one thing at a time.

The only keyboard behavior I've observed while the keyboard is open is that Escape closes the dropdown, and the Arrow Up / Down keys open the dropdown (when it's vertical, I'd assume Left and Right would when horizontal).

tl;dr - The function that's supposed to add list keyboard handlers to the Combobox (createSelectableCollection()) doesn't seem to functionally do anything with the keyboard except specifically the thing I don't want it to do and there's no way to override or control this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant