-
Notifications
You must be signed in to change notification settings - Fork 841
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
[EuiDatePicker] Better handle onBlur
callback
#5441
Conversation
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 did a quick run through with Safari and Chrome, looking specifically at keyboard and click navigation patterns. They all worked well, and honored the blur()
event to hide the tooltip.
Preview documentation changes for this PR: https://eui.elastic.co/pr_5441/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5441/ |
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've taken a pass through with Firefox, Chrome, and Edge on MacOS locally to test for consistent behavior with the tooltip. All were behaving as expected. LGTM!
Preview documentation changes for this PR: https://eui.elastic.co/pr_5441/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5441/ |
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 looks good, Greg! I ran through the changes in the PR preview and compared it to the current version of the docs. The updated onBlur
functionality creates a clean experience.
This reverts commit 70dc65b.
Preview documentation changes for this PR: https://eui.elastic.co/pr_5441/ |
Summary
Addresses a specific bug whereby wrapping
EuiDatePicker
inEuiToolTip
will cause the tooltip to never close once opened: https://codesandbox.io/s/nervous-smoke-kk5ysThe root problem was that provided
onBlur
callbacks are simply ignored, and as such the solution applies more broadly to allblur
event scenarios.The intended behavior is that
onBlur
callbacks only get called when the entire component loses focus. That is, if either the input or any element in the popover has focus, we treat that as internal focus transfer and notblur
.Additional update includes a change to
EuiToolTip
so that is does not omit configuredonBlur
andonFocus
callbacks on the child component.Checklist