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

A11y test for rules and connectors #140665

Merged
merged 23 commits into from
Nov 16, 2022
Merged

Conversation

bhavyarm
Copy link
Contributor

@bhavyarm bhavyarm commented Sep 13, 2022

@bhavyarm bhavyarm self-assigned this Sep 13, 2022
@bhavyarm bhavyarm added Project:Accessibility Feature:Actions/ConnectorTypes Issues related to specific Connector Types on the Actions Framework Feature:Alerting/RulesManagement Issues related to the Rules Management UX Feature:Actions/ConnectorsManagement Issues related to Connectors Management UX v8.5.0 release_note:skip Skip the PR/issue when compiling release notes test_xpack_functional and removed Feature:Actions/ConnectorTypes Issues related to specific Connector Types on the Actions Framework labels Sep 13, 2022
@bhavyarm bhavyarm force-pushed the rulesConnectorsA11yTest branch from 6adae77 to 0912d05 Compare September 13, 2022 22:19
@bhavyarm
Copy link
Contributor Author

@1Copenut I have a few more a11y errors on this app along with these two bugs which I already logged. I am going to ping you for a zoom to see about the failures in this test and if they are valid a11y issues. Thanks!

Here are the screenshots -
Screen Shot 2022-09-21 at 9 19 58 AM

Screen Shot 2022-09-21 at 9 20 26 AM

Screen Shot 2022-09-21 at 9 37 14 AM

@bhavyarm bhavyarm force-pushed the rulesConnectorsA11yTest branch from 963f1a6 to a5f9981 Compare September 22, 2022 20:43
@bhavyarm bhavyarm mentioned this pull request Sep 28, 2022
77 tasks
@bhavyarm
Copy link
Contributor Author

bhavyarm commented Nov 8, 2022

@1Copenut this is ready for you to debug. Good luck. Thanks :)

@bhavyarm bhavyarm marked this pull request as ready for review November 16, 2022 14:44
@elasticmachine
Copy link
Contributor

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

@bhavyarm bhavyarm requested review from 1Copenut and LeeDr November 16, 2022 14:44
Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

LGTM! I left a comment about further findings on the Rules Panel block we've been troubleshooting.

await testSubjects.click('notifyWhenSelect');
await testSubjects.click('onActiveAlert');
await testSubjects.click('solutionsFilterButton');
await a11y.testAppSnapshot();
Copy link
Contributor

Choose a reason for hiding this comment

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

@bhavyarm Agree on skipping this block for now. I talked with Chandler about this issue yesterday and have a few ideas I'm still exploring.

When I moved the await a11y.testAppSnapshot() to the top of the block, it caused the first input to throw an error and the "one rule must be applied" message to appear, which I would not expect to happen. The accessibility test should not be affecting the DOM, so I'm looking more closely at the helper itself. Is it injecting a script, and is the block under test using mutation observer in the React code. I'll gather what I can find and open an investigation issue with Kibana Ops.

it.skip('a11y test on inputs on rules panel', async () => {
  await a11y.testAppSnapshot();
  // Causes the first input to lose focus, throw error code
  // Also causes the "one rule must be applied" error to appear
  await testSubjects.click('ruleNameInput');
  await testSubjects.setValue('ruleNameInput', 'testRule');
  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much @1Copenut

@bhavyarm bhavyarm added the backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) label Nov 16, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @bhavyarm

@bhavyarm bhavyarm merged commit d84a36d into elastic:main Nov 16, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 16, 2022
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5
8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 16, 2022
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 17, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@bhavyarm bhavyarm removed backport missing Added to PRs automatically when the are determined to be missing a backport. backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) labels Nov 21, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 22, 2022
@bhavyarm bhavyarm removed the v8.5.0 label Nov 23, 2022
@kibanamachine kibanamachine added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Actions/ConnectorsManagement Issues related to Connectors Management UX Feature:Alerting/RulesManagement Issues related to the Rules Management UX Project:Accessibility release_note:skip Skip the PR/issue when compiling release notes test_xpack_functional v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants