feat(axe-core-4.6): Expose axe-core relatedNodes as "Related paths" card row #6388
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JGibson2019
approved these changes
Feb 1, 2023
7 tasks
dbjorge
added a commit
that referenced
this pull request
Feb 2, 2023
#### Details This PR adds the new "Related paths" field introduced in #6388 to the output of selecting "Copy failure details" from a card. The way this functionality works is mostly shared with how issue filing works. However, this PR intentionally sets up issue filing to not include relatedPaths (using a new `isLengthConstrained` option to `issueDetailsBuilder`) because we found in testing that including relatedPaths there would reliably trip the URL length limit for both ADO and GH issues. Per discussion with @ferBonnin , this PR consistently omits relatedPaths from ADO/GH issues, and consistently includes them (where available) in copied failure details. <details> <summary>Example of copy failure details output before the change:</summary> ``` Title: WCAG 1.3.1: Ensures elements with an ARIA role that require child roles contain them (div[data-focuszone-id="FocusZone179"]) Tags: Accessibility, WCAG 1.3.1, aria-required-children Issue: Ensures elements with an ARIA role that require child roles contain them (aria-required-children - https://accessibilityinsights.io/info-examples/web/aria-required-children) Target application: Fluent UI - Controls - React - DetailsList - https://developer.microsoft.com/en-us/fluentui#/controls/web/detailslist Element path: div[data-focuszone-id="FocusZone179"] Snippet: <div role="row" class="ms-FocusZone css-134 ms-DetailsHeader root-297" data-automationid="DetailsHeader" data-focuszone-id="FocusZone179"> How to fix: Fix any of the following: Element has children which are not allowed (see related nodes) Element has no aria-busy="true" attribute Environment: Microsoft Edge version 109.0.1518.70 ==== This accessibility issue was found using Accessibility Insights for Web 2023.1.31.2331 (axe-core 4.6.3), a tool that helps find and fix accessibility issues. Get more information & download this tool at http://aka.ms/AccessibilityInsights. ``` </details> <details> <summary>after the change (only difference is new Related paths section):</summary> ``` Title: WCAG 1.3.1: Ensures elements with an ARIA role that require child roles contain them (div[data-focuszone-id="FocusZone179"]) Tags: Accessibility, WCAG 1.3.1, aria-required-children Issue: Ensures elements with an ARIA role that require child roles contain them (aria-required-children - https://accessibilityinsights.io/info-examples/web/aria-required-children) Target application: Fluent UI - Controls - React - DetailsList - https://developer.microsoft.com/en-us/fluentui#/controls/web/detailslist Element path: div[data-focuszone-id="FocusZone179"] Snippet: <div role="row" class="ms-FocusZone css-134 ms-DetailsHeader root-297" data-automationid="DetailsHeader" data-focuszone-id="FocusZone179"> Related paths: div[data-focuszone-id="FocusZone179"] > .cellSizerStart-305.ms-DetailsHeader-cellSizer[data-sizer-index="1"] div[data-focuszone-id="FocusZone179"] > .cellSizerStart-305.ms-DetailsHeader-cellSizer[data-sizer-index="2"] .cellSizerStart-305.ms-DetailsHeader-cellSizer[data-sizer-index="3"] div[data-sizer-index="4"] How to fix: Fix any of the following: Element has children which are not allowed (see related nodes) Element has no aria-busy="true" attribute Environment: Microsoft Edge version 109.0.1518.70 ==== This accessibility issue was found using Accessibility Insights for Web 1.0.4 (axe-core 4.6.3), a tool that helps find and fix accessibility issues. Get more information & download this tool at http://aka.ms/AccessibilityInsights. ``` </details> <details> <summary>after the change for a different example without any relatedNodes available (no change in format vs before):</summary> ``` Title: WCAG 1.4.3: Ensures the contrast between foreground and background colors meets WCAG 2 AA contrast ratio thresholds (code) Tags: Accessibility, WCAG 1.4.3, color-contrast Issue: Ensures the contrast between foreground and background colors meets WCAG 2 AA contrast ratio thresholds (color-contrast - https://accessibilityinsights.io/info-examples/web/color-contrast) Target application: Fluent UI - Controls - React - DetailsList - https://developer.microsoft.com/en-us/fluentui#/controls/web/detailslist Element path: .css-344 > .css-167 > .root-238.ms-Link[target="_blank"] > code Snippet: <code>IBaseProps</code> How to fix: Fix any of the following: Element has insufficient color contrast of 4.04 (foreground color: #0078d4, background color: #f2f2f2, font size: 8.3pt (11px), font weight: normal). Expected contrast ratio of 4.5:1 Environment: Microsoft Edge version 109.0.1518.70 ==== This accessibility issue was found using Accessibility Insights for Web 1.0.4 (axe-core 4.6.3), a tool that helps find and fix accessibility issues. Get more information & download this tool at http://aka.ms/AccessibilityInsights. ``` </details> Screenshot of GitHub issue filed for a case with relatedNodes present, demonstrating that it's omitted from the filed issue body:  ##### Motivation Makes "Copy failure details" consistent with new card display behavior (feedback from #6388). ##### Context I considered just not implementing `relatedPaths` in the HTML and Markdown formatters, which would have had the same effect with a bit less code as the solution I chose using `isLengthConstrained`. But I didn't like that as much in terms of separation of concerns; it didn't feel like the formatters' responsibility to understand that they were being used in the context of a service with a length limit or not. #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: 2020748 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [x] (UI changes only) Added ~screenshots/GIFs~text output to description above - [x] (UI changes only) Verified usability with NVDA/JAWS
dbjorge
added a commit
to microsoft/accessibility-insights-service
that referenced
this pull request
Feb 2, 2023
…lock, meta-viewport, and "Related paths" in reports (#2383) #### Details This PR: * Updates `axe-core` to v4.6.3 * Updates `accessibility-insights-report` to corresponding v4.6.3 * Removes `link-in-text-block` and `meta-viewport` from the rule exclusion list, matching the new AI4Web behavior enabling them as automated checks * Enables a new report feature to include "Related paths" for failure cards of axe results with "relatedNodes" info by propogating this axe property in `combined-report-data-converter.ts` * Bumps the `accessibility-insights-scan` minor version to `1.6.0` in anticipation of a corresponding release for that package Screenshot of combined report segment with new Related paths UX generated using the following CLI command in this PR branch: `node C:\repos\accessibility-insights-service\packages\cli\dist\ai-scan-cli.js --crawl --url https://developer.microsoft.com/en-us/fluentui#/controls/web/detailslist --maxUrls 5`  ##### Motivation Keep service rules version in sync with AI4Web, which has begun release validation today for its corresponding release (2.37.0) ##### Context * [web 4.6.3 PR (#6334)](microsoft/accessibility-insights-web#6334) * [web "Related paths" PR with screenshots of new report UI (#6388)](microsoft/accessibility-insights-web#6388) #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: 2017450 - [x] Added relevant unit test for your changes. (`yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] Ran precheckin (`yarn precheckin`) - [ ] Validated in an Azure resource group
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details
This PR implements a feature to add a new "Related paths" row to result cards that represent axe-core results which included any check results with
relatedNodesproperties.It adds this information to:
Screenshot of FastPass Automated Checks showing an
aria-required-childrenviolation with Related paths:Screenshot of FastPass Report Export showing the same info:
Screenshot of FastPass Automated Checks'
color-contrastresults on a page whereaxe-coreonly reports related paths for some results - note that cards for results where the data is present show it, and results where axe-core didn't report that data omit it.Screenshot of combined report with related path field:
Motivation
This is primarily to improve the actionability of the
aria-required-children"unknown" violation path which we added support for during our 4.6.3 update in #6334. That path uses a how to fix message of the form"Element has children which are not allowed (see related nodes)", which isn't very actionable without this PR since we don't expose "related nodes" anywhere. This specific message might be updated in a future version of axe-core (see dequelabs/axe-core#3842)This secondarily improves the actionability of a few other rules that axe-core exposes related nodes for - particularly,
color-contrastviolations where axe knows the node that is responsible for the background color of a violation will now present that background node as a "related path".Context
In addition to this change, we'll want to make a docs change to https://accessibilityinsights.io/info-examples/web/aria-required-children/ to make this specific form of
aria-required-childrenviolation easier for a user to understand and take action on.Pull request checklist
yarn fastpassyarn test)<rootDir>/test-results/unit/coveragefix:,chore:,feat(feature-name):,refactor:). SeeCONTRIBUTING.md.