Fixed EuiSuperDatePicker input focus. #3154
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
ashikmeerankutty
changed the title
|
@cchaos Thanks for the review. On investigating, the issue was the focus event from the input is bubbled and the tab content divs on focus is called which in turn calls the focus on the tab button. It also found that this issue only occurs for input elements. I have fixed the issue by filtering the onFocus events from the |
|
Hmm, I still don't think this solution is quite right. It's not general enough to allow any content to receive the focus like buttons or select elements. I think the problem lies where the focus is being listened for. The |
|
@cchaos Thanks for the suggestion. By moving the focus to the ref to EuiTabs fixed the issue. |
|
Jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3154/ |
| expand={expand} | ||
| display={display} | ||
| size={size} | ||
| onFocus={this.initializeFocus}> |
There was a problem hiding this comment.
@ashikmeerankutty I talked with @chandlerprall and we came to the conclusion that the divRef is still need in order to allow tabbing through all of the tabs. But that it should also be move to <EuiTabs>. By doing so you'll most likely need to modify EuiTabs to forwardRef to the containing div.
There was a problem hiding this comment.
@cchaos Thanks for the comment. Moved divRef to EuiTabs. Do I need to rename divRef to tabRef for readability?
There was a problem hiding this comment.
I think that would help a lot. Perhaps even tabsRef (plural) so it's more obvious that it's on the whole EuiTabs component.
|
Jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3154/ |
There was a problem hiding this comment.
I checked through IE, FF, and Chrome and the original problem is fixed with IE still behaving correctly. 👍 🎉 Thank you @ashikmeerankutty , our Firefox users will be very appreciative!
I did notice that there is an existing bug where using the mouse to select a different tab than the initially selected, the focus should be on the one clicked not the one selected. I will log a new issue for this since it was not created by this PR.
|
retest |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3154/ |
|
Hmm, possibly flakey? retest |
|
retest |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3154/ |
Summary
Fixes #2490
This was due to the tab component overriding the focus of the input. This was fixed by overriding the tab components focus when onMouseDown on input.
Before
After
Checklist
- [ ] Props have proper autodocs- [ ] Added documentation examples- [ ] Checked for accessibility including keyboard-only and screenreader modes