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

Failing test: X-Pack Accessibility Tests - Group 2.x-pack/test/accessibility/apps/group2/ml·ts - X-Pack Accessibility Tests - Group 2 ml Accessibility for user with full ML access with data loaded data frame analytics outlier job exploration page #172598

Closed
kibanamachine opened this issue Dec 5, 2023 · 13 comments
Assignees
Labels
failed-test A test failure on a tracked branch, potentially flaky-test :ml

Comments

@kibanamachine
Copy link
Contributor

kibanamachine commented Dec 5, 2023

A test failed on a tracked branch

Error: a11y report:

VIOLATION
  [aria-hidden-focus]: Ensures aria-hidden elements are not focusable nor contain focusable elements
    Impact: serious
    Help: https://dequeuniversity.com/rules/axe/4.8/aria-hidden-focus?application=axeAPI
    Elements:
      - <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" class="euiIcon eui-alignTop css-1h8nuic-euiIcon-s-isLoading" tabindex="0" role="img" aria-label="Info" aria-hidden="true" data-is-loading="true"></svg>
      - <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" class="euiIcon eui-alignTop css-1h8nuic-euiIcon-s-isLoading" tabindex="0" role="img" aria-label="Info" aria-hidden="true" data-is-loading="true"></svg>
      - <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" class="euiIcon eui-alignTop css-1h8nuic-euiIcon-s-isLoading" tabindex="0" role="img" aria-label="Info" aria-hidden="true" data-is-loading="true"></svg>
      - <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" class="euiIcon eui-alignTop css-1h8nuic-euiIcon-s-isLoading" tabindex="0" role="img" aria-label="Info" aria-hidden="true" data-is-loading="true"></svg>
      - <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" class="euiIcon css-1349ml7-euiIcon-l-isLoading" tabindex="0" role="img" aria-label="Info" aria-hidden="true" data-is-loading="true"></svg>
    at AccessibilityService.assertValidAxeReport (a11y.ts:75:13)
    at AccessibilityService.testAppSnapshot (a11y.ts:48:10)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at Context.<anonymous> (ml.ts:123:11)
    at Object.apply (wrap_function.js:73:16)

First failure: CI Build - main

@kibanamachine kibanamachine added the failed-test A test failure on a tracked branch, potentially flaky-test label Dec 5, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Dec 5, 2023
@botelastic botelastic bot removed the needs-team Issues missing a team label label Dec 5, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@mistic
Copy link
Member

mistic commented Dec 5, 2023

Skipped.

main: 27d8fa3

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@peteharverson
Copy link
Contributor

Removing the blocker label - the results page is loading successfully, but looks like the loading indicator for the scatterplot matrix is failing the aria-hidden-focus accessibility test.

Will look into a fix.

@peteharverson peteharverson self-assigned this Dec 6, 2023
@peteharverson
Copy link
Contributor

From discussion with @1Copenut , EuiIcon is adding a tabindex automatically when icons are rendered, and the test is trying to evaluate a hidden element with a tabindex, and throwing an error. Can look to improve the rendered output on the EUI side to use isLoaded prop to apply the tabindex late instead of immediately.

@1Copenut
Copy link
Contributor

1Copenut commented Dec 6, 2023

I filed a feature request in EUI, outlining the problem ( elastic/eui#7400 ). EUI will discuss this is grooming on Monday and decide a way forward. Looking at the nested components, it wasn't as straight forward as passing the tabindex late, but there are a few other options we can try in EUI to future-proof against this violation.

@peteharverson
Copy link
Contributor

Moving to 8.13.0, awaiting a fix for elastic/eui#7400.

@peteharverson
Copy link
Contributor

Moving to 8.14.0, still awaiting a fix for elastic/eui#7400.

@cee-chen
Copy link
Contributor

cee-chen commented Mar 19, 2024

Rather than waiting for EUI to solve this, could y'all consider updating the failing FTR test(s) to wait for all/any .euiIcon/[data-icon-type]s on the page to no longer have the [data-is-loading] attribute before running your axe check?

@cee-chen
Copy link
Contributor

Pete super kindly talked through the problem with me on Slack and we agreed it's a net a11y benefit to not hide icons with aria-labels to screen readers regardless of whether they're empty/loading or not. Fix should be coming in #7606

@1Copenut
Copy link
Contributor

Pete super kindly talked through the problem with me on Slack and we agreed it's a net a11y benefit to not hide icons with aria-labels to screen readers regardless of whether they're empty/loading or not. Fix should be coming in #7606

I like this approach a lot. Thank you both for collaborating on a strong solution and fix. Just approved the EUI PR.

@peteharverson
Copy link
Contributor

Closing, with tests unskipped in #179833 following changes made to EuiIcon in elastic/eui#7606.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
failed-test A test failure on a tracked branch, potentially flaky-test :ml
Projects
None yet
Development

No branches or pull requests

6 participants