-
Notifications
You must be signed in to change notification settings - Fork 317
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 interfering click handlers #9127
Conversation
@@ -43,7 +43,7 @@ const props = defineProps<{ | |||
|
|||
const emit = defineEmits<{ | |||
accepted: [searcherExpression: string, requiredImports: RequiredImport[]] | |||
closed: [searcherExpression: string, requiredImports: RequiredImport[]] | |||
closed: [searcherExpression: string, requiredImports: RequiredImport[], anyInputChange: boolean] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a closed
event? I think the CB could just emit accepted
or canceled
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Closed" means that it lost focus (due to background click, for example). Since this PR, it does differ from direct accept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I don't see the need to increase the complexity of the ComponentBrowser/GraphEditor interface by exposing this distinction to the GraphEditor. The CB could decide whether to emit accepted
or canceled
internally, and keep anyInputChange
encapsulated.
Pull Request Description
Fixes one failure in #8942 which caught a real issue: clicks at various panels were triggering many handlers at once, often unexpectedly - for example quick clicking at breadcrumbs (and automatic tests always click fast) we could also trigger getting out the current node.
Therefore, I added
stopPropagation
for all mouse events on "panel" level + where I think the event should be considered "handled" and no longer bother anyone. Also, to unify things, most actions are forclick
event.Additionally I spotted and fixed some issues:
operatorX.
nodes)click
event.Important Notes
I removed
PointerMain
binding for deselectAll, because it was triggered every time did the area selection.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.