-
Notifications
You must be signed in to change notification settings - Fork 1
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: Fixes bug where clicking on the canvas did not remove focus from other elements #539
Conversation
|
Coverage Report
File CoverageNo changed files found. |
69d6087
to
5c590dd
Compare
// Prevent the default behavior for mouse clicks that would cause text | ||
// selection, but keep the behavior where focus is removed from other | ||
// elements. | ||
event.preventDefault(); |
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.
This was what was causing the bug, but event.preventDefault
is still mostly desired behavior. Simulate the focus behavior by blurring the active element in the document.
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.
I don't know much about the world of MouseEvents, but looks good to me! I appreciate the comments in the code :)
// elements. | ||
event.preventDefault(); | ||
if (document.activeElement instanceof HTMLElement) { | ||
document.activeElement.blur(); |
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.
will it blur itself too?
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.
Ooh, good question. I don't think blurring itself would do anything because it's not an element that has focus behavior or styling, but I'll add a check in and skip blurring if it's the same element. Thank you!
Problem
Closes #535, "clicking on viewport should remove focus from other elements."
Estimated review size: tiny, <5 minutes
Solution
Type of change