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

[SelectionAutoCompleteInput] Fix clicking on auto-complete result. #26844

Merged
merged 4 commits into from
Jan 6, 2025

Conversation

salazarm
Copy link
Contributor

@salazarm salazarm commented Jan 6, 2025

Summary & Motivation

Clicking on a result was broken because our focus handler would cause us to call showHint again which would cause the completion to be thrown out. To fix this guard against calling showHint again if hints are already visible.

Copy link

github-actions bot commented Jan 6, 2025

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-dtgnswnqf-elementl.vercel.app
https://salazarm-fix-clicking-result.core-storybook.dagster-docs.io

Built with commit 4f07f14.
This pull request is being automatically deployed with vercel-action

Comment on lines 209 to 212
if (document.body.querySelector('.CodeMirror-hints')) {
// Hints already visible
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could future proof this a bit by specifying a custom container for the hints... in case we ever render multiple of these inputs on the same page and specify closeOnUnfocuse: false

Copy link
Member

Choose a reason for hiding this comment

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

Hm, yeah. I could see there being more than one of these on a page.

Comment on lines 209 to 212
if (document.body.querySelector('.CodeMirror-hints')) {
// Hints already visible
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

Hm, yeah. I could see there being more than one of these on a page.

@salazarm salazarm merged commit 763b7a7 into master Jan 6, 2025
1 of 2 checks passed
@salazarm salazarm deleted the salazarm/fix-clicking-result branch January 6, 2025 19:18
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