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

[Lens] (Accessibility) Suggestions have no focus state and no accessible name #83602

Closed
myasonik opened this issue Nov 17, 2020 · 8 comments · Fixed by #84653
Closed

[Lens] (Accessibility) Suggestions have no focus state and no accessible name #83602

myasonik opened this issue Nov 17, 2020 · 8 comments · Fixed by #84653
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Lens Project:Accessibility Team:Visualizations Visualization editors, elastic-charts and infrastructure WCAG A

Comments

@myasonik
Copy link
Contributor

The problem

  1. Suggestions at the bottom of the page have no apparent focus state when tabbing though them.
  2. Suggestions have no accessible name (technically, the "Current" one does have an accessible name, though it's just "Current" which doesn't really give you the info you need).

The fix

  1. Add a focus state. (If these are EUI buttons, they should already so I'm not sure what happened. If these are not EUI buttons, why not?)

  2. Add an aria-label with the chart type. Add aria-current="true" to the button that is currently selected as well.

    Bonus points: These really shouldn't be buttons are at all - it's a radio group. Ideally, radio inputs would control this under the hood and would visually be replaces by whatever you want (this is how single selection button groups work in EUI too).

One more thing

The selected state for the current suggestion looks a lot like (is?) the EUI focus state for a lot of controls. This is pretty confusing when other things on the page are focused and will probably become more confusing when focus states are returned to the other suggestions.

Relevant WCAG Criteria:

  1. 2.4.7 Focus visible - Level AA
  2. 1.1.1 Non-text content - Level A
@myasonik myasonik added bug Fixes for quality problems that affect the customer experience Project:Accessibility WCAG A Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Nov 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-accessibility (Project:Accessibility)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293 flash1293 changed the title (Accessibility) Suggestions have no focus state and no accessible name [Lens] (Accessibility) Suggestions have no focus state and no accessible name Nov 18, 2020
@flash1293
Copy link
Contributor

@MichaelMarcialis Can you suggest another style for the currently active suggestion which is not looking like the regular EUI focus state so we can use that one for focus?

This is how it looks like right now:
Screenshot 2020-11-23 at 18 12 39

The currently active suggestion is using the blue outline normally used for the focused element, but the actually focused element (the donut chart) is only shown by a very slight y translation.

@mbondyra
Copy link
Contributor

mbondyra commented Dec 1, 2020

Hi @flash1293 @MichaelMarcialis @myasonik, I submitted a draft PR with one of the solutions – we could either:

  1. leave the focus ring to mark current visualization and use EuiButton focus styles to show focus state (that's what I do in this PR, gif below)
    focus_suggestions

  2. Come up with new styles for active suggestion just like @flash1293 proposes and use either a. default EuiButton focus styles or b. focusRing styles

Let me know your thoughts!

@flash1293
Copy link
Contributor

I like the default focus state if we get rid of the underline text decoration (so just darker background)

@MichaelMarcialis
Copy link
Contributor

MichaelMarcialis commented Dec 1, 2020

@flash1293, @mbondyra: I've worked up a quick concept for the suggestions focus states. Have a look at the links below and let me know if you're good with this direction. Happy to hop on a Zoom as well, if desired.

Suggested changes include:

  • For consistency with other EUI elements, the focus styles should apply the EUI focus ring to all suggestion options as shown here, rather than using it to indicate the currently selected visualization.
  • In order to account for the above focus ring change and better differentiate between currently selected visualizations and other suggestions, the current selection now gets an EUI lightest shade background and no box-shadow to give it a look of being pressed-in and selected.

Focus Second

As a side note, I noticed that selecting one of the suggestion options doesn't currently move the "Current" text to that new selection. Is this intentional or is it a bug?

@flash1293
Copy link
Contributor

flash1293 commented Dec 2, 2020

I don't have a strong preference, both seem equally fine to me. Based on the interaction to get into these states I don't think either is confusing to the user.

As a side note, I noticed that selecting one of the suggestion options doesn't currently move the "Current" text to that new selection. Is this intentional or is it a bug?

This is intentional, if a suggestion is selected you are in a "preview" state and can still go back to your "real" current visualization. The actual suggestions are always based on the "Current" visualization to the left. As soon as you start editing or press the refresh button to the right of the suggestion, it "commits" the suggestion and it becomes the new "current", providing you with a new set of suggestions based on the current visualization.

This confused some people before, I'm happy if we can come up with a better flow here, but I would like to keep it off this PR and treat it with a low priority.

@mbondyra
Copy link
Contributor

mbondyra commented Dec 3, 2020

@myasonik the amount of !importants (6 and I still didn't style it properly) I have to add to make this work as EuiButton doesn't make it worth. In fact we override almost all the styles. I'll leave it as EuiPanel that sematically is also a button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Lens Project:Accessibility Team:Visualizations Visualization editors, elastic-charts and infrastructure WCAG A
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants