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

[EuiDataGrid] Expansion popover initial focus #5683

Closed
cchaos opened this issue Mar 3, 2022 · 2 comments · Fixed by #5784
Closed

[EuiDataGrid] Expansion popover initial focus #5683

cchaos opened this issue Mar 3, 2022 · 2 comments · Fixed by #5784

Comments

@cchaos
Copy link
Contributor

cchaos commented Mar 3, 2022

I'd like to discuss the behavior of cell popovers' initial focus behavior. To be honest, this may even apply more broadly to EuiPopover in general, but the problem is very highlighted in the EuiDataGrid.

When clicking on a cell with simple text, or a single interactive element, or plain text with a cell action, the behavior feels good to initially focus on the first focusable element.
Screen Shot 2022-03-03 at 09 52 59 AM
Screen Shot 2022-03-03 at 09 52 52 AM
Screen Shot 2022-03-03 at 09 53 14 AM

The problem

Where it breaks down is when there is more complex cell contents, like in the row heights example.

When there's an interactive element within the cell contents that possibly very far down, that's what gets focused and therefore the popover auto-scrolls to this element:

Screen Shot 2022-03-02 at 17 29 24 PM

This isn't great for the specific reason that it feels buggy to not show the start of the contents.

Possible solution: Auto-focus the popover instead

It's already part of the tab order, just make it the first focused item. We might want to consider how we handle the visible focus ring, but this feels much clearer and expected.
Screen Shot 2022-03-03 at 09 46 53 AM

Pros:

  • For complex content this will keep the start of the content visible
  • Screen readers should be able to now read the full cell contents instead of jumping to a random link within
  • For simple content with cell actions, screen-readers will read the full cell contents instead of jumping to the cell action which may not provide context

Cons:

  • For simple content, the extra focus may be visually distracting
  • ??
@cee-chen
Copy link
Contributor

cee-chen commented Mar 3, 2022

++ to this proposal - I've always found it strange that we auto-jump to the first focusable item in the popover instead of showing the contents of the entire popover. @1Copenut are there any accessibility/screen reader recommendations for this that we're missing?

@1Copenut
Copy link
Contributor

1Copenut commented Mar 4, 2022

There are potential a11y concerns with setting focus other than on the first text element inside a popover. In the description example moving focus to the Sri Lanka link as soon as the popover is opened may lead to missed context. Users might not always be ingesting the cell text "South Maude" immediately and then opening the popover.

I'd like to explore setting focus on the popover itself consistently. This can be verbose where users will hear the helper text about being in a modal dialog and to press Escape to close. I've observed users moving quickly to the next text element when they hear this familiar message because they've normalized the modal UI and that the content they're interested in is one or two keypresses further on.

@zuhairmahd please feel free to further or revise this proposal if there's an option or consideration I've overlooked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants