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

(Accessibility) Routed tabs cause multiple failures #77828

Closed
myasonik opened this issue Sep 17, 2020 · 10 comments
Closed

(Accessibility) Routed tabs cause multiple failures #77828

myasonik opened this issue Sep 17, 2020 · 10 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature loe:medium Medium Level of Effort Project:Accessibility skipped-test Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services WCAG A

Comments

@myasonik
Copy link
Contributor

myasonik commented Sep 17, 2020

Issue

Routed tabs cause a WCAG A failure in several ways:

  • It breaks tabs accessibility because the button[role="tab"] is not a direct decedent of [role="tablist"]
  • It breaks tab focus order because the <button> inside the <a> causes a secondary tab stop on the same-ish element

These failures are causing a test failure in our a11y testing suite (test skipped).

Recommended fix

These tabs are only visually tabs and aren't semantically tabs. It's just some links for a navigation. EUI should be flexible enough to render this visually as tabs but semantically as navigation (by removing the role attributes and not rendering the <button>). (If not, open issue against EUI.)

@myasonik myasonik added bug Fixes for quality problems that affect the customer experience Project:Accessibility WCAG A Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services skipped-test labels Sep 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@smith
Copy link
Contributor

smith commented Jul 25, 2023

One place where it looks like we're doing this is here:

https://github.com/smith/kibana/blob/6b65e909356a8f9e9f29db28ac8c41edae9959e1/x-pack/plugins/infra/public/components/asset_details/links/tab_to_uptime.tsx#L27-L34

There may be others.

@cee-chen looked at this with me on Slack in #accessibility and said:

TBH though, I'm not totally sure there's anything you can do without EUI updating our component to render a

instead of a [role="tablist"] (edit: actually, passing hrefs and not just onClicks might help) (edited)

Please check with her when addressing this to make sure there's something we can do.

@smith smith added the Feature:Metrics UI Metrics UI feature label Jul 25, 2023
@crespocarlos crespocarlos self-assigned this Jul 28, 2023
@crespocarlos
Copy link
Contributor

@smith, this is an old ticket. Apparently, things have changed.

image image



It breaks tabs accessibility because the button[role="tab"] is not a direct decedent of [role="tablist"]

Based on the screenshots above, that's not the case. button[role="tab"] is a direct descendant of [role="tablist"]. Same for a[role="tab"]

It breaks tab focus order because the inside the causes a secondary tab stop on the same-ish element

I couldn't find any example of that.

Besides, I ran the test mentioned on the description and it passed

@cee-chen
Copy link
Contributor

I can confirm that tab focus order is no longer an issue - that appears to have been fixed in EUI.

That being said, a[role="tab"] is still not a perfect screen reader experience, since screen readers announce it as a tabbed item and not as a link, but you could mitigate that by adding an aria-label to your external link icon before the "APM"/"Uptime" text that reads like "This is an external link to another page". With that change, I would personally consider this issue resolved.

@smith
Copy link
Contributor

smith commented Jul 28, 2023

I think the best solution here would be:

  • Change the "tabs" on the asset details flyout to be just a list of links. There's no reason for them to be tabs, and they don't open in a new window.
  • Leave the inventory alone, where the "tabs" are part of the actual tab list. We'll be replacing that with the asset details flyout very soon.

@cee-chen
Copy link
Contributor

cee-chen commented Jul 28, 2023

@smith I think you and @crespocarlos are referring to two different UI items:

256818908-63116544-55a0-4ff3-afa6-c7ed15472021

I agree with Nathan that the two items that look like plain EuiButtonEmptys should just be buttons and not tabs, but the actual tabs would be more accessible with a very quick aria-label improvement.

@smith
Copy link
Contributor

smith commented Jul 28, 2023

@cee-chen
Copy link
Contributor

Ahh gotcha. Thanks for clarifying! I'd definitely like to see the top right "tabs" not be tabs!

@crespocarlos
Copy link
Contributor

The top right links are in fact EuiButtonEmpty:

APM: https://github.com/elastic/kibana/blob/v8.9.0/x-pack/plugins/infra/public/components/asset_details/links/link_to_apm_services.tsx
Uptime: https://github.com/elastic/kibana/blob/v8.9.0/x-pack/plugins/infra/public/components/asset_details/links/link_to_uptime.tsx.

Image

@smith, the asset details code has APM and Uptime links as tabs too. They were moved to the asset details when we started moving stuff from the node details flyout from the Inventory UI, but they are not currently in use.

@smith
Copy link
Contributor

smith commented Jul 31, 2023

Thanks for looking @crespocarlos. I'm going to close this now.

@smith smith closed this as completed Jul 31, 2023
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:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature loe:medium Medium Level of Effort Project:Accessibility skipped-test Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services WCAG A
Projects
None yet
Development

No branches or pull requests

7 participants