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

Added autoFocus option to EuiTabbedContent #2062

Merged
merged 3 commits into from
Jun 20, 2019

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jun 19, 2019

The focus state of EuiTabs can confuse users since it's so prominent compared to the selected state. Some users thing the focus state is the selected state. This PR attempts to allow setting the initial focus of the tabbed items from initial (first tab) to selected (selected tab).

There are 2 issues with it at the moment:

  • [ ] 1. If the selected tab is not the first, users have to shift + tab in order to get to the previous tabs. Got confirmation from most that this is not an issue.
  • 2. I'm currently using document.getElementById() to find the selected tab, which probably isn't great
  • 3. The way I have it setup is that when a user focuses in on the EuiTabbedContent, it sets the state.focus to true and then auto focuses on the selected tab. The problem is that I have to reset the state to false when a user tabs out of the control. However, it seems to cycle as blur then focus each tab press.


Also fixes the second action item for the EuiDatePicker from GAH:

Checklist

  • [ ] This was checked in mobile
  • This was checked in IE11
  • [ ] This was checked in dark mode
  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • [ ] This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • This was checked against keyboard-only and screenreader scenarios
  • [ ] This required updates to Framer X components

@cchaos
Copy link
Contributor Author

cchaos commented Jun 19, 2019

Opening this up for review. The "issue" item, is still unchecked mainly to see if anyone hollers about any concern over it. Otherwise, it's not really an issue and this should be good to go.

@cchaos cchaos marked this pull request as ready for review June 19, 2019 20:20
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Code looks good; tested locally, too.

I don't think the shift + tab thing is a problem at all. The focus state is clear enough and that is the normal interaction for reverse focus order

@cchaos cchaos changed the title [WIP] Added autoFocus option to EuiTabbedContent Added autoFocus option to EuiTabbedContent Jun 20, 2019
@cchaos
Copy link
Contributor Author

cchaos commented Jun 20, 2019

I'm merging this one in because I want to make sure it gets into K7.3

@cchaos cchaos merged commit c107d3a into elastic:master Jun 20, 2019
@cchaos cchaos deleted the tab-change-initial-focus branch June 20, 2019 18:00
@cchaos cchaos mentioned this pull request Apr 3, 2020
6 tasks
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.

3 participants