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

fix: No longer possible to prevent trigger #139

Merged
merged 1 commit into from
Dec 13, 2021
Merged

fix: No longer possible to prevent trigger #139

merged 1 commit into from
Dec 13, 2021

Conversation

tommoor
Copy link
Collaborator

@tommoor tommoor commented Dec 13, 2021

Previously you could prevent command bar from appearing by preventing default elsewhere in the host application. Since this change:

87db5bb#diff-d5312972da4ac728a05d23c05e373b0e32c1241c3381805a1f1e4a160449b0e7L50-L52

useCapture means it is no longer possible to prevent command bar from appearing.

@vercel
Copy link

vercel bot commented Dec 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/timc/kbar/J625Zf1jowUr3R9pqXbn4K2oNTN2
✅ Preview: https://kbar-git-tommoor-patch-1-timc.vercel.app

@tommoor tommoor requested a review from timc1 December 13, 2021 03:01
@timc1
Copy link
Owner

timc1 commented Dec 13, 2021

@tommoor you'll still run into the issue when the command bar is opened on top of a modal, and hitting ESC will propagate and close the modal as well right?

@tommoor
Copy link
Collaborator Author

tommoor commented Dec 13, 2021

At least in Outline it isn't an issue because opening CMD+K steals focus away from the underlying modal

@timc1
Copy link
Owner

timc1 commented Dec 13, 2021

I suppose that's the proper way of handling modals and dialogs in the first place 👍

@timc1 timc1 merged commit c0b53b7 into main Dec 13, 2021
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.

2 participants