Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations #6910

dtaylor113 · 2020-10-12T21:18:14Z

Root cause was the <div id="modal-container">...</div> was getting a aria-hidden=true attribute which hide it and it's children (modal header, footer, body) from accesibility tools. A11y violation due to form fields within modal could have focus, but they were hidden.

Changed element passed to react-modal's setAppElement(...) to be 'app-container' instead of 'modal-container'. setAppElement(...) will set aria-hidden=true for the element (and all it's children!) and 'hide' them from accesibility tools. This is done in order to 'hide' app elements behind the open modal, and will toggle/remove aria-hidden once the modal is closed. So previously setting AppElement to the modal was hiding the modal and it's contents from accesiblity tools! This fix requires app-container and modal-container to be sibling elements, and not child-parent.

This fix allowed the modal contents to be included in accesibility tools and thus, once fixed, raised another violation:

Running:  crud/namespace-crud.spec.ts
  Namespace
  ...
1 accessibility violation was detected for Delete Namespace modal
┌─────────┬─────────┬────────────┬──────────────────────────────────────────┬───────┐
│ (index) │   id    │   impact   │               description                │ nodes │
├─────────┼─────────┼────────────┼──────────────────────────────────────────┼───────┤
│    0    │ 'label' │ 'critical' │ 'Ensures every form element has a label' │   1   │
└─────────┴─────────┴────────────┴──────────────────────────────────────────┴───────┘

This required adding aria-label to a form element.

This PR will also cause the CI to fail upon detection of a11y violations!!

Running test-cypress.sh reported no a11y violoations, so going forward, any violations will need to be fixed prior to PR merge.

CC @jessiehuff

openshift-ci-robot · 2020-10-12T21:21:34Z

@dtaylor113: This pull request references Bugzilla bug 1885702, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-10-13T14:50:50Z

@dtaylor113: This pull request references Bugzilla bug 1885702, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-10-13T14:52:09Z

@dtaylor113: This pull request references Bugzilla bug 1885702, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

spadgett

Thanks @dtaylor113

spadgett · 2020-10-13T14:46:52Z

frontend/public/components/modals/delete-namespace-modal.jsx

@@ -51,6 +51,7 @@ class DeleteNamespaceModal extends PromiseComponent {
            className="pf-c-form-control"
            onKeyUp={this._matchTypedNamespace}
            placeholder="Enter name"
+            aria-label="Enter name of namespace to delete"


Suggested change

aria-label="Enter name of namespace to delete"

aria-label={`Enter the name of the ${this.props.kind.label} to delete`}

openshift-ci-robot · 2020-10-13T14:56:19Z

@dtaylor113: This pull request references Bugzilla bug 1885702, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

…t instead of 'modal-container'. setAppElement(...) will set 'aria-hidden=true' in attempt to hide app behind the open modal.

spadgett

/lgtm

openshift-ci-robot · 2020-10-13T16:33:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dtaylor113 · 2020-10-14T14:13:40Z

/retest

jessiehuff · 2020-10-14T14:45:40Z

Really great work on this, @dtaylor113!!! 🎉 I just tested this in VO, and it's significantly better than before. Before these changes, I was unable to interact with the modal outside of the input. I could not navigate to the header, the text, or the footer buttons- I was only able to interact with the input which did not give much context to what was happening either. I was also able to interact with the rest of the page which should have been hidden from the user while interacting with the modal. With these changes, when the user opens the modal now, they aren't confused by the rest of the page, and you added more context with the aria label. My focus goes to the input, it explains what I should do, and if I want more info, I can go up and read the rest of the modal. Awesome update! 😄

dtaylor113 · 2020-10-14T14:49:51Z

Really great work on this, @dtaylor113!!! I just tested this in VO...

@jessiehuff Thanks! Really cool that you can experience what these changes actually do. :-)

spadgett · 2020-10-14T17:24:32Z

/refresh

openshift-ci-robot · 2020-10-14T19:29:33Z

@dtaylor113: All pull requests linked via external trackers have merged:

openshift/console#6910

Bugzilla bug 1885702 has been moved to the MODIFIED state.

In response to this:

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

spadgett · 2020-10-14T20:21:51Z

/cherry-pick release-4.6

openshift-cherrypick-robot · 2020-10-14T20:21:59Z

@spadgett: new pull request created: #6931

In response to this:

/cherry-pick release-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dtaylor113 requested a review from spadgett October 12, 2020 21:18

openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Oct 12, 2020

openshift-ci-robot requested review from jcaianirh and jeff-phillips-18 October 12, 2020 21:18

openshift-ci-robot added the component/core Related to console core functionality label Oct 12, 2020

openshift deleted a comment from openshift-ci-robot Oct 12, 2020

spadgett reviewed Oct 13, 2020

View reviewed changes

Changed react-modal's setAppElement(...) to be 'app-container' elemen…

4a148a9

…t instead of 'modal-container'. setAppElement(...) will set 'aria-hidden=true' in attempt to hide app behind the open modal.

dtaylor113 force-pushed the a11y-fix-aria-hidden-focus branch from d8a54b0 to 4a148a9 Compare October 13, 2020 16:04

spadgett approved these changes Oct 13, 2020

View reviewed changes

openshift-ci-robot assigned spadgett Oct 13, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 13, 2020

openshift-merge-robot merged commit e9d99ac into openshift:master Oct 14, 2020

openshift-cherrypick-robot mentioned this pull request Oct 14, 2020

[release-4.6] Bug 1888430: Cypress: Fix 'aria-hidden-focus' accesibility violations #6931

Closed

This was referenced Oct 16, 2020

Bug 1878873 : cypress-cucumber-automation-framework #6776

Merged

Bug 1888985: Fix Cypress test flake and accesibility violation: 'Ensures buttons have discernible text' #6951

Merged

spadgett added this to the v4.7 milestone Oct 21, 2020

dtaylor113 mentioned this pull request Nov 12, 2020

Cypress fixes: 'Create Project' timing issue and temporarily disable a11y check for 'aria-hidden-focus' #7193

Merged

dtaylor113 deleted the a11y-fix-aria-hidden-focus branch November 25, 2020 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations #6910

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations #6910

dtaylor113 commented Oct 12, 2020 •

edited

openshift-ci-robot commented Oct 12, 2020

openshift-ci-robot commented Oct 13, 2020

openshift-ci-robot commented Oct 13, 2020

spadgett left a comment

spadgett Oct 13, 2020

dtaylor113 Oct 13, 2020

openshift-ci-robot commented Oct 13, 2020

spadgett left a comment

openshift-ci-robot commented Oct 13, 2020

dtaylor113 commented Oct 14, 2020

jessiehuff commented Oct 14, 2020

dtaylor113 commented Oct 14, 2020

spadgett commented Oct 14, 2020

openshift-ci-robot commented Oct 14, 2020

spadgett commented Oct 14, 2020

openshift-cherrypick-robot commented Oct 14, 2020

	aria-label="Enter name of namespace to delete"
	aria-label={`Enter the name of the ${this.props.kind.label} to delete`}

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations #6910

Bug 1885702: Cypress: Fix 'aria-hidden-focus' accesibility violations #6910

Conversation

dtaylor113 commented Oct 12, 2020 • edited

This PR will also cause the CI to fail upon detection of a11y violations!!

openshift-ci-robot commented Oct 12, 2020

openshift-ci-robot commented Oct 13, 2020

openshift-ci-robot commented Oct 13, 2020

spadgett left a comment

Choose a reason for hiding this comment

spadgett Oct 13, 2020

Choose a reason for hiding this comment

dtaylor113 Oct 13, 2020

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 13, 2020

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 13, 2020

dtaylor113 commented Oct 14, 2020

jessiehuff commented Oct 14, 2020

dtaylor113 commented Oct 14, 2020

spadgett commented Oct 14, 2020

openshift-ci-robot commented Oct 14, 2020

spadgett commented Oct 14, 2020

openshift-cherrypick-robot commented Oct 14, 2020

dtaylor113 commented Oct 12, 2020 •

edited