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

Disable select item with mouse #116

Merged
merged 7 commits into from
Jan 30, 2024

Conversation

joaom00
Copy link
Contributor

@joaom00 joaom00 commented Apr 11, 2023

As discussed in #49 keyboard-only item selection is the new default. I didn't add a prop to enable mouse selection because this can be implemented by the consumer if he wants to. What do you think?

I think the only problem here is that the mouse selection style will break for users so would this be a breaking change?

Adding a new prop disablePointerSelection to disable item selection via pointer events

@vercel
Copy link

vercel bot commented Apr 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cmdk-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 3:45am

@phil-loops
Copy link

phil-loops commented Apr 14, 2023

Played around on the demo site in safari. Seems to have solved a big chunk of the woes on that browser!

What would you think of having the mouse behavior controlled by a new prop? Might be able to keep the old styling, but enable new opt-in behavior.

@joaom00
Copy link
Contributor Author

joaom00 commented Apr 16, 2023

I'm in favor of letting the user make the selection with mouse but it can require a slight boilerplate so enabling this behavior with a prop is not a bad thing. I believe that the correct approach here is to disable the mouse selection with a prop and make this the default in a future v1 release

@trymbill
Copy link

trymbill commented May 4, 2023

Would love to see this merged. Feels a lot more intuitive and less prone to accidental selections.

@joaom00
Copy link
Contributor Author

joaom00 commented May 9, 2023

@pacocoursey would like to know what you think!

Copy link
Owner

@pacocoursey pacocoursey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@pacocoursey pacocoursey merged commit e97839f into pacocoursey:main Jan 30, 2024
2 of 3 checks passed
@Just-Moh-it
Copy link

@pacocoursey not sure if this made it to the 0.2.1 release. Seems like this was merged the same day the release 👀

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

Successfully merging this pull request may close these issues.

5 participants