pacocoursey / cmdk

Fast, unstyled command menu React component.
https://cmdk.paco.me
MIT License
9.56k stars 274 forks source link

Option to not change selected item via mouse events #49

Open pomdtr opened 2 years ago

pomdtr commented 2 years ago

I'm building a raycast clone for linux as a learning project, using cmdk and wails.io. It worked really well for me so far, thanks a lot for releasing this !

One pain point I have is the keyboard/mouse interaction during the selection. When I scroll a list using a keyboard, the mouse is often stealing the focus.

https://user-images.githubusercontent.com/17577332/188937337-b5bd1537-d63d-47b0-9e75-8008ea5fb222.mov

I love how raycast solve the issue: the mouse only provide an hover effect, and only select on click.

https://user-images.githubusercontent.com/17577332/188936938-39d7154b-770a-4338-be39-5ee5dbe2d4d9.mov

Would you be interested in adding this behaviour to the app ? I can work on the pr.

pacocoursey commented 2 years ago

Interesting use case! I think I'd be open to offering this behavior. Want to take a stab at it through a new prop passed to Command? Not sure of the name, maybe something like disablePointerSelections: boolean

raunofreiberg commented 2 years ago

Actually feels like a bug to me and the Raycast behavior seems desired. Should this be the default?

pacocoursey commented 2 years ago

I'm not so sure, both React Aria (Combobox) and Radix UI (Dropdown) combine "mouse selection" with "keyboard selection" rather than separating them. However, Aria Combobox patterns keep them distinct.

Personally I find using the mouse "finicky" to select items, sometimes your pointer just happens to move (you bump the table, it slips on mousepad, or is generally bad) and it's annoying when that has adverse effects. That's one of the reasons I disabled scroll-into-view from pointer events in #3. So I'd be in favor of making this the new default.

peduarte commented 1 year ago

I personally agree that selecting an item via hover is a bit optimistic. Would love to be able to disable that, or even have a bit more control based on the actual event via some sort of onSelect prop (similar to how Radix does with its Item components). By receiving the Event, users can decide to preventDefault, or even select on double click (event.detail === 2)

@pomdtr also, what are you building? curious 😄 DM me on Twitter if you wanna talk about it!

trymbill commented 1 year ago

+1 on this. Feels like a hover should just be a hover and a mouse click should be the same as a keyboard navigation event. Anyone working on this currently? If not I might try and carve out some time to see if I can contribute.

mahendradambe commented 1 year ago

Here are the following things which should be considered before moving to implementation

role=option (mdn)

All selectable options should have aria-selected match their state, true when selected and false when not. If an option is not selectable, aria-selected can be omitted. A disabled option can have aria-disabled="true" and aria-selected="false" to communicate to the user that the option is present, albeit disabled.

Implementing the above would mean that there would can be which can be non-selectable and would only execute an action.

There can be two approaches:

  1. Not manage the state and let the library consumer set relevant prop on <Command.Item /> (i.e., selected) This approach is easier to start off with and solves the current issue.

  2. Manage the selection state and provide options for controlled/uncontrolled approach by exposing relevant props on <Command /> (i.e., value, onValueChange) and <Command.Item /> (selectable) For this approach we might want to consider all the possibilities involving the scope of aria-selectable, i.e., can only one item be selectable per list or per group? There might also be a requirement for aria-multiselectable which might need to be fulfilled.