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

Add shouldIgnoreViolation optional prop to Scenario for a11y tests #12

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Nov 8, 2023

Context

We have recently found an accessibility test that was failing dues to axe-core not allowing a very specific combination of elements, specifically a button with combobox role linked with a label using its for attribute.

While the specification says this is a valid combination, there's a specific screen reader which does not play well with it, and that's why axe-core reports a violation.

We have tested the combination with other screen readers (VoiceOver and orca), and the elements get announced as expected, so we want to be able to ignore this violation.

Changes

This PR introduces a new shouldIgnoreViolation callback that can be provided as part of every scenario passed to checkAccessibility.

This callback is invoked with every violation reported by axe-core, allowing downstream code to determine which violations are relevant and which are not.

I decided to use this approach instead of just providing the ids of violations to ignore, because then we could end up ignoring actual violations by mistake.

With this, downstream code has all the information about the violation in order to decide whether it has to be ignored or not.

How to use

As a POC, I have used this PR in client in the test where this was first spotted, so it will look more or less like this:

-  it(
-    'should pass a11y checks',
-    checkAccessibility({
-      content: () => createComponent(),
-    }),
-  );
+  it('should pass a11y checks', async () => {
+    const wrapper = createComponent();
+    const select = await waitForElement(wrapper, SelectNext);
+
+    await checkAccessibility({
+      content: () => wrapper,
+      shouldIgnoreViolation: ({ id, nodes }) => {
+        if (id !== 'button-name') {
+          return false;
+        }
+
+        // axe-core can report a violation on any button[role="combobox"] linked
+        // to a label if it does not have aria-label or aria-labelledby, because
+        // the Dragon NaturallySpeaking screen reader does not play well with
+        // that combination.
+        // Since its use is marginal, we want to ignore the "button-name"
+        // violation if reported on SelectNext.
+        // See https://github.com/dequelabs/axe-core/issues/4235 for context
+
+        const targets = nodes.flatMap(node => node.target);
+        return targets.includes(`#${select.prop('buttonId')}`);
+      },
+    })();
+  });

@acelaya acelaya force-pushed the a11y-violations-filtering branch from 187e192 to d240c17 Compare November 8, 2023 09:57
@acelaya acelaya changed the title Add isViolationRelevant optional prop to Scenario for a11y tests Add shouldIgnoreViolation optional prop to Scenario for a11y tests Nov 8, 2023
@acelaya acelaya merged commit 62515d7 into main Nov 8, 2023
@acelaya acelaya deleted the a11y-violations-filtering branch November 8, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants