From 74bcefe6b9e87d7439749e480af0ad90346a9924 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Thu, 26 Jan 2023 15:38:41 -0500 Subject: [PATCH 01/11] implement Related paths card row --- .../cards/related-paths-card-row.scss | 6 ++++ .../cards/related-paths-card-row.tsx | 35 +++++++++++++++++++ .../components/fix-instruction-processor.tsx | 2 ++ ...unified-result-property-configurations.tsx | 20 +++++------ .../store-data/unified-data-interface.ts | 1 + .../scan-results-to-unified-results.ts | 25 +++++++++++-- src/scanner/iruleresults.d.ts | 6 ++++ 7 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 src/common/components/cards/related-paths-card-row.scss create mode 100644 src/common/components/cards/related-paths-card-row.tsx diff --git a/src/common/components/cards/related-paths-card-row.scss b/src/common/components/cards/related-paths-card-row.scss new file mode 100644 index 0000000000..1f29d79e61 --- /dev/null +++ b/src/common/components/cards/related-paths-card-row.scss @@ -0,0 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +.path-list { + padding-left: 16px; + list-style-type: disc; +} diff --git a/src/common/components/cards/related-paths-card-row.tsx b/src/common/components/cards/related-paths-card-row.tsx new file mode 100644 index 0000000000..d2303e055b --- /dev/null +++ b/src/common/components/cards/related-paths-card-row.tsx @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { SimpleCardRow } from 'common/components/cards/simple-card-row'; +import { CardRowProps } from 'common/configs/unified-result-property-configurations'; +import { NamedFC } from 'common/react/named-fc'; +import { isEmpty } from 'lodash'; +import * as React from 'react'; +import styles from './related-paths-card-row.scss'; + +export interface RelatedPathsCardRowProps extends CardRowProps { + propertyData: string[]; +} + +export const RelatedPathsCardRow = NamedFC( + 'RichResolutionCardRow', + ({ index, propertyData }) => { + if (isEmpty(propertyData)) { + return null; + } + + return ( + + {propertyData.map(selector => ( +
  • {selector}
    • + ))} + + } + rowKey={`related-paths-row-${index}`} + /> + ); + }, +); diff --git a/src/common/components/fix-instruction-processor.tsx b/src/common/components/fix-instruction-processor.tsx index fee3f63456..3f9d3222e2 100644 --- a/src/common/components/fix-instruction-processor.tsx +++ b/src/common/components/fix-instruction-processor.tsx @@ -46,6 +46,8 @@ export class FixInstructionProcessor { private readonly originalMiddleSentence = ' and the original foreground color: '; public process(fixInstruction: string, recommendColor: RecommendColor): JSX.Element { + fixInstruction = fixInstruction.replace(/\(see related nodes\)/g, '(see related paths)'); + const matches = this.getColorMatches(fixInstruction); let recommendationSentences: string[] = []; diff --git a/src/common/configs/unified-result-property-configurations.tsx b/src/common/configs/unified-result-property-configurations.tsx index cc509a662b..6ae3cb36d1 100644 --- a/src/common/configs/unified-result-property-configurations.tsx +++ b/src/common/configs/unified-result-property-configurations.tsx @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { ClassNameCardRow } from 'common/components/cards/class-name-card-row'; import { ContentDescriptionCardRow } from 'common/components/cards/content-description-card-row'; +import { RelatedPathsCardRow } from 'common/components/cards/related-paths-card-row'; import { RichResolutionCardRow } from 'common/components/cards/rich-resolution-card-row'; import { TextCardRow } from 'common/components/cards/text-card-row'; import { UrlsCardRow } from 'common/components/cards/urls-card-row'; @@ -13,15 +14,7 @@ import { SnippetCardRow } from '../components/cards/snippet-card-row'; import { FixInstructionProcessor } from '../components/fix-instruction-processor'; import { ReactFCWithDisplayName } from '../react/named-fc'; -export type PropertyType = - | 'css-selector' - | 'how-to-fix-web' - | 'richResolution' - | 'snippet' - | 'className' - | 'contentDescription' - | 'text'; -export const AllPropertyTypes: PropertyType[] = [ +export const AllPropertyTypes = [ 'css-selector', 'how-to-fix-web', 'richResolution', @@ -29,7 +22,9 @@ export const AllPropertyTypes: PropertyType[] = [ 'className', 'contentDescription', 'text', -]; + 'relatedCssSelectors', +] as const; +export type PropertyType = (typeof AllPropertyTypes)[number]; export interface CardRowDeps { fixInstructionProcessor: FixInstructionProcessor; @@ -59,6 +54,10 @@ export const cssSelectorConfiguration: PropertyConfiguration = { cardRow: PathCardRow, }; +export const relatedCssSelectorsConfiguration: PropertyConfiguration = { + cardRow: RelatedPathsCardRow, +}; + export const snippetConfiguration: PropertyConfiguration = { cardRow: SnippetCardRow, }; @@ -91,6 +90,7 @@ const propertyIdToConfigurationMap: PropertyIdToConfigurationMap = { contentDescription: contentDescriptionConfiguration, text: textConfiguration, urls: urlsConfiguration, + relatedCssSelectors: relatedCssSelectorsConfiguration, }; export function getPropertyConfiguration(id: string): Readonly { diff --git a/src/common/types/store-data/unified-data-interface.ts b/src/common/types/store-data/unified-data-interface.ts index ae240bb6d3..366fdf0581 100644 --- a/src/common/types/store-data/unified-data-interface.ts +++ b/src/common/types/store-data/unified-data-interface.ts @@ -93,6 +93,7 @@ export type UnifiedIdentifiers = { export type UnifiedDescriptors = { snippet?: string; boundingRectangle?: BoundingRectangle; + relatedCssSelectors?: string[]; } & InstancePropertyBag; export type UnifiedRichResolution = { diff --git a/src/injected/adapters/scan-results-to-unified-results.ts b/src/injected/adapters/scan-results-to-unified-results.ts index 13ccb137e3..8839e97ec4 100644 --- a/src/injected/adapters/scan-results-to-unified-results.ts +++ b/src/injected/adapters/scan-results-to-unified-results.ts @@ -8,7 +8,7 @@ import { UnifiedResult, } from '../../common/types/store-data/unified-data-interface'; import { UUIDGenerator } from '../../common/uid-generator'; -import { AxeNodeResult, RuleResult, ScanResults } from '../../scanner/iruleresults'; +import { AxeNodeResult, RuleResult, ScanResults, Target } from '../../scanner/iruleresults'; import { IssueFilingUrlStringUtils } from './../../issue-filing/common/issue-filing-url-string-utils'; export type ConvertScanResultsToUnifiedResultsDelegate = ( @@ -111,7 +111,7 @@ export class ConvertScanResultsToUnifiedResults { ruleResultData: RuleResultData, getResolution: ResolutionCreator, ): UnifiedResult => { - const cssSelector = nodeResult.target.join(';'); + const cssSelector = this.nodeToCssSelector(nodeResult); return { uid: this.uuidGenerator(), status: ruleResultData.status, @@ -124,6 +124,7 @@ export class ConvertScanResultsToUnifiedResults { }, descriptors: { snippet: nodeResult.snippet || nodeResult.html, + relatedCssSelectors: this.relatedCssSelectors(nodeResult), }, resolution: { howToFixSummary: nodeResult.failureSummary!, @@ -131,4 +132,24 @@ export class ConvertScanResultsToUnifiedResults { }, }; }; + + private relatedCssSelectors(nodeResult: AxeNodeResult): string[] | undefined { + const output: string[] = []; + for (const checkType of ['all', 'any', 'none'] as const) { + const checks = nodeResult[checkType]; + for (const check of checks) { + const relatedSelectors = check.relatedNodes?.map(this.nodeToCssSelector) ?? []; + output.push(...relatedSelectors); + } + } + return output.length === 0 ? undefined : output; + } + + private nodeToCssSelector = (node: { target: Target }): string => { + const { target } = node; + if (typeof target === 'string') { + return target; + } + return target.join(';'); + }; } diff --git a/src/scanner/iruleresults.d.ts b/src/scanner/iruleresults.d.ts index 85f7911b33..252dd10ff3 100644 --- a/src/scanner/iruleresults.d.ts +++ b/src/scanner/iruleresults.d.ts @@ -69,6 +69,12 @@ export interface FormattedCheckResult { message: string; data: IAxeCheckResultExtraData; result?: boolean; + relatedNodes?: AxeRelatedNode[]; +} + +export interface AxeRelatedNode { + target: string[]; + html: string; } export interface IAxeCheckResultExtraData { From aab51ee43ae29c2a5eb936a48fcc578857215c00 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Fri, 27 Jan 2023 16:25:20 -0500 Subject: [PATCH 02/11] add unit tests for new extraction functionality --- .../adapters/extract-related-selectors.ts | 31 ++++ .../scan-results-to-unified-results.ts | 33 +--- src/injected/main-window-initializer.ts | 2 + src/reports/package/reporter-factory.ts | 8 +- src/scanner/iruleresults.d.ts | 2 +- ...an-results-to-unified-results.test.ts.snap | 24 +++ .../extract-related-selectors.test.ts | 160 ++++++++++++++++++ .../scan-results-to-unified-results.test.ts | 4 + 8 files changed, 237 insertions(+), 27 deletions(-) create mode 100644 src/injected/adapters/extract-related-selectors.ts create mode 100644 src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts diff --git a/src/injected/adapters/extract-related-selectors.ts b/src/injected/adapters/extract-related-selectors.ts new file mode 100644 index 0000000000..35638a97e6 --- /dev/null +++ b/src/injected/adapters/extract-related-selectors.ts @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { TargetHelper } from 'common/target-helper'; +import { uniq } from 'lodash'; +import { AxeNodeResult } from '../../scanner/iruleresults'; + +export type RelatedSelectorExtractor = typeof extractRelatedSelectors; + +export function extractRelatedSelectors(nodeResult: AxeNodeResult): string[] | undefined { + let output: string[] = []; + for (const checkType of ['all', 'any', 'none'] as const) { + const checks = nodeResult[checkType]; + for (const check of checks) { + const relatedSelectors = + check.relatedNodes?.map(n => TargetHelper.getSelectorFromTarget(n.target)) ?? []; + output.push(...relatedSelectors); + } + } + + // This doesn't usually change anything; it would mainly affect if a rule had multiple + // checks that each returned overlapping sets of related nodes. Axe generally returns + // nodes in a reasonable order, so we want to use a method that preserves order here. + output = uniq(output); + + // This does have a practical impact; some axe checks (eg, color-contrast) will occasionally + // return the node itself as a "related node", and we'd rather avoid this. + const selfSelector = TargetHelper.getSelectorFromTarget(nodeResult.target); + output = output.filter(relatedSelector => relatedSelector !== selfSelector); + + return output.length === 0 ? undefined : output; +} diff --git a/src/injected/adapters/scan-results-to-unified-results.ts b/src/injected/adapters/scan-results-to-unified-results.ts index 8839e97ec4..f64f6dbd68 100644 --- a/src/injected/adapters/scan-results-to-unified-results.ts +++ b/src/injected/adapters/scan-results-to-unified-results.ts @@ -1,5 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { TargetHelper } from 'common/target-helper'; +import { RelatedSelectorExtractor } from 'injected/adapters/extract-related-selectors'; import { ResolutionCreator } from 'injected/adapters/resolution-creator'; import { flatMap } from 'lodash'; @@ -22,9 +24,10 @@ interface RuleResultData { export class ConvertScanResultsToUnifiedResults { constructor( - private uuidGenerator: UUIDGenerator, - private getFixResolution: ResolutionCreator, - private getCheckResolution: ResolutionCreator, + private readonly uuidGenerator: UUIDGenerator, + private readonly getFixResolution: ResolutionCreator, + private readonly getCheckResolution: ResolutionCreator, + private readonly extractRelatedSelectors: RelatedSelectorExtractor, ) {} public automatedChecksConversion: ConvertScanResultsToUnifiedResultsDelegate = ( @@ -111,7 +114,7 @@ export class ConvertScanResultsToUnifiedResults { ruleResultData: RuleResultData, getResolution: ResolutionCreator, ): UnifiedResult => { - const cssSelector = this.nodeToCssSelector(nodeResult); + const cssSelector = TargetHelper.getSelectorFromTarget(nodeResult.target); return { uid: this.uuidGenerator(), status: ruleResultData.status, @@ -124,7 +127,7 @@ export class ConvertScanResultsToUnifiedResults { }, descriptors: { snippet: nodeResult.snippet || nodeResult.html, - relatedCssSelectors: this.relatedCssSelectors(nodeResult), + relatedCssSelectors: this.extractRelatedSelectors(nodeResult), }, resolution: { howToFixSummary: nodeResult.failureSummary!, @@ -132,24 +135,4 @@ export class ConvertScanResultsToUnifiedResults { }, }; }; - - private relatedCssSelectors(nodeResult: AxeNodeResult): string[] | undefined { - const output: string[] = []; - for (const checkType of ['all', 'any', 'none'] as const) { - const checks = nodeResult[checkType]; - for (const check of checks) { - const relatedSelectors = check.relatedNodes?.map(this.nodeToCssSelector) ?? []; - output.push(...relatedSelectors); - } - } - return output.length === 0 ? undefined : output; - } - - private nodeToCssSelector = (node: { target: Target }): string => { - const { target } = node; - if (typeof target === 'string') { - return target; - } - return target.join(';'); - }; } diff --git a/src/injected/main-window-initializer.ts b/src/injected/main-window-initializer.ts index c132c62545..ff5d750396 100644 --- a/src/injected/main-window-initializer.ts +++ b/src/injected/main-window-initializer.ts @@ -14,6 +14,7 @@ import { UnifiedScanResultStoreData } from 'common/types/store-data/unified-data import { toolName } from 'content/strings/application'; import { TabStopRequirementActionMessageCreator } from 'DetailsView/actions/tab-stop-requirement-action-message-creator'; import { GetDetailsSwitcherNavConfiguration } from 'DetailsView/components/details-view-switcher-nav'; +import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; import { getCheckResolution, getFixResolution } from 'injected/adapters/resolution-creator'; import { filterNeedsReviewResults } from 'injected/analyzers/filter-results'; import { NotificationTextCreator } from 'injected/analyzers/notification-text-creator'; @@ -293,6 +294,7 @@ export class MainWindowInitializer extends WindowInitializer { generateUID, getFixResolution, getCheckResolution, + extractRelatedSelectors, ); const notificationTextCreator = new NotificationTextCreator(scanIncompleteWarningDetector); diff --git a/src/reports/package/reporter-factory.ts b/src/reports/package/reporter-factory.ts index 0bb3ca43ac..7007b350d0 100644 --- a/src/reports/package/reporter-factory.ts +++ b/src/reports/package/reporter-factory.ts @@ -7,6 +7,7 @@ import { getA11yInsightsWebRuleUrl } from 'common/configs/a11y-insights-rule-res import { CardSelectionViewData } from 'common/get-card-selection-view-data'; import { getCardViewData } from 'common/rule-based-view-model-provider'; import { generateUID } from 'common/uid-generator'; +import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; import { getCheckResolution, getFixResolution } from 'injected/adapters/resolution-creator'; import { ConvertScanResultsToUnifiedResults } from 'injected/adapters/scan-results-to-unified-results'; import { convertScanResultsToUnifiedRules } from 'injected/adapters/scan-results-to-unified-rules'; @@ -94,7 +95,12 @@ const axeResultsReportGenerator = (parameters: AxeReportParameters) => { mapAxeTagsToGuidanceLinks, ruleProcessor, ); - const getUnifiedResults = new ConvertScanResultsToUnifiedResults(generateUID, getFixResolution, getCheckResolution).automatedChecksConversion; + const getUnifiedResults = new ConvertScanResultsToUnifiedResults( + generateUID, + getFixResolution, + getCheckResolution, + extractRelatedSelectors + ).automatedChecksConversion; const deps: AxeResultsReportDeps = { reportHtmlGenerator, diff --git a/src/scanner/iruleresults.d.ts b/src/scanner/iruleresults.d.ts index 252dd10ff3..c8b90b22f8 100644 --- a/src/scanner/iruleresults.d.ts +++ b/src/scanner/iruleresults.d.ts @@ -73,7 +73,7 @@ export interface FormattedCheckResult { } export interface AxeRelatedNode { - target: string[]; + target: (string | string[])[]; html: string; } diff --git a/src/tests/unit/tests/injected/adapters/__snapshots__/scan-results-to-unified-results.test.ts.snap b/src/tests/unit/tests/injected/adapters/__snapshots__/scan-results-to-unified-results.test.ts.snap index 2ad80e9b62..c6987361cc 100644 --- a/src/tests/unit/tests/injected/adapters/__snapshots__/scan-results-to-unified-results.test.ts.snap +++ b/src/tests/unit/tests/injected/adapters/__snapshots__/scan-results-to-unified-results.test.ts.snap @@ -6,6 +6,9 @@ exports[`ScanResults to Unified Results Test automaticChecksConversion works wit [ { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for target1,id1", + ], "snippet": "html1", }, "identifiers": { @@ -27,6 +30,9 @@ exports[`ScanResults to Unified Results Test automaticChecksConversion works wit }, { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for target2,id2", + ], "snippet": "html2", }, "identifiers": { @@ -48,6 +54,9 @@ exports[`ScanResults to Unified Results Test automaticChecksConversion works wit }, { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for target3,id3", + ], "snippet": "html3", }, "identifiers": { @@ -69,6 +78,9 @@ exports[`ScanResults to Unified Results Test automaticChecksConversion works wit }, { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for passTarget1,passTarget2", + ], "snippet": "html-pass", }, "identifiers": { @@ -97,6 +109,9 @@ exports[`ScanResults to Unified Results Test needsReviewConversion works with fi [ { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for incompleteTarget1", + ], "snippet": "html-incomplete", }, "identifiers": { @@ -117,6 +132,9 @@ exports[`ScanResults to Unified Results Test needsReviewConversion works with fi }, { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for target1,id1", + ], "snippet": "html1", }, "identifiers": { @@ -138,6 +156,9 @@ exports[`ScanResults to Unified Results Test needsReviewConversion works with fi }, { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for target2,id2", + ], "snippet": "html2", }, "identifiers": { @@ -159,6 +180,9 @@ exports[`ScanResults to Unified Results Test needsReviewConversion works with fi }, { "descriptors": { + "relatedCssSelectors": [ + "extractRelatedSelectors output for target3,id3", + ], "snippet": "html3", }, "identifiers": { diff --git a/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts b/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts new file mode 100644 index 0000000000..d41d352b3c --- /dev/null +++ b/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts @@ -0,0 +1,160 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; +import { AxeNodeResult, FormattedCheckResult, Target } from 'scanner/iruleresults'; + +describe(extractRelatedSelectors, () => { + function checkResult(relatedTargets?: Target[]): FormattedCheckResult { + const result: FormattedCheckResult = { + id: 'check-id', + message: 'check-message', + data: {}, + }; + if (relatedTargets != null) { + result.relatedNodes = relatedTargets.map(target => ({ html: 'related-html', target })); + } + return result; + } + it('returns undefined if there are no relatedNodes', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [checkResult()], + any: [checkResult()], + none: [], + }; + const output = extractRelatedSelectors(input); + expect(output).toBeUndefined(); + }); + + it('returns undefined if the only relatedNodes are the node itself', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [], + any: [checkResult([['#node']])], + none: [checkResult([['#node']])], + }; + const output = extractRelatedSelectors(input); + expect(output).toBeUndefined(); + }); + + it('extracts as-is simple targets per relatedNode', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [checkResult([['#related-1']])], + any: [], + none: [checkResult([['.related-2']])], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual(['#related-1', '.related-2']); + }); + + it('semicolon-joins array-style targets per relatedNode', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [], + any: [checkResult([['#related-1-outer', '#related-1-inner']])], + none: [], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual(['#related-1-outer;#related-1-inner']); + }); + + it('comma and semicolon joins array-of-array-style targets per relatedNode', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [], + any: [ + checkResult([ + [ + ['#related-1-outer-1', '#related-1-outer-2'], + ['#related-1-inner-1', '#related-1-inner-2'], + ], + ]), + ], + none: [], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual([ + '#related-1-outer-1,#related-1-outer-2;#related-1-inner-1,#related-1-inner-2', + ]); + }); + + it('includes multiple relatedNodes from same checks', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [checkResult([['#related-1'], ['#related-2'], ['#related-3']])], + any: [], + none: [], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual(['#related-1', '#related-2', '#related-3']); + }); + + it('includes relatedNodes from multiple different checks', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [checkResult([['#related-1']]), checkResult([['#related-2']])], + any: [], + none: [checkResult([['#related-3']])], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual(['#related-1', '#related-2', '#related-3']); + }); + + it('omits duplicates', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [checkResult([['#related-1']]), checkResult([['#related-2']])], + any: [checkResult([['#related-2']]), checkResult([['#related-3']])], + none: [checkResult([['#related-3']]), checkResult([['#related-1']])], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual(['#related-1', '#related-2', '#related-3']); + }); + + it.each([[[1, 2, 3]], [[3, 2, 1]], [[1, 3, 2]]])( + 'maintains original order', + (order: number[]) => { + const relatedSelectors = order.map(n => `#related-${n}`); + + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [ + checkResult([[relatedSelectors[0]]]), + checkResult([[relatedSelectors[1]]]), + checkResult([[relatedSelectors[2]]]), + ], + any: [], + none: [], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual([ + relatedSelectors[0], + relatedSelectors[1], + relatedSelectors[2], + ]); + }, + ); + + it('omits the node itself', () => { + const input: AxeNodeResult = { + target: ['#node'], + html: 'node-html', + all: [checkResult([['#related-1']]), checkResult([['#node']])], + any: [], + none: [checkResult([['#related-2']])], + }; + const output = extractRelatedSelectors(input); + expect(output).toStrictEqual(['#related-1', '#related-2']); + }); +}); diff --git a/src/tests/unit/tests/injected/adapters/scan-results-to-unified-results.test.ts b/src/tests/unit/tests/injected/adapters/scan-results-to-unified-results.test.ts index daa5d57593..d3d1844f30 100644 --- a/src/tests/unit/tests/injected/adapters/scan-results-to-unified-results.test.ts +++ b/src/tests/unit/tests/injected/adapters/scan-results-to-unified-results.test.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { RelatedSelectorExtractor } from 'injected/adapters/extract-related-selectors'; import { ResolutionCreator } from 'injected/adapters/resolution-creator'; import { ConvertScanResultsToUnifiedResults } from 'injected/adapters/scan-results-to-unified-results'; import { IMock, Mock, MockBehavior, Times } from 'typemoq'; @@ -12,6 +13,7 @@ describe('ScanResults to Unified Results Test', () => { let generateGuidMock: IMock<() => string>; let fixResolutionCreatorMock: IMock; let checkResolutionCreatorMock: IMock; + let extractRelatedSelectorsStub: RelatedSelectorExtractor; beforeEach(() => { const guidStub = 'gguid-mock-stub'; @@ -22,6 +24,7 @@ describe('ScanResults to Unified Results Test', () => { .verifiable(Times.atLeastOnce()); fixResolutionCreatorMock = Mock.ofType(); checkResolutionCreatorMock = Mock.ofType(); + extractRelatedSelectorsStub = node => [`extractRelatedSelectors output for ${node.target}`]; }); const nullIdentifiers = [null, undefined, {}]; @@ -110,6 +113,7 @@ describe('ScanResults to Unified Results Test', () => { generateGuidMock.object, fixResolutionCreatorMock.object, checkResolutionCreatorMock.object, + extractRelatedSelectorsStub, ); } From 392fcc2795051c4b1fcb13d16bf89154bfe72847 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Mon, 30 Jan 2023 15:21:27 -0500 Subject: [PATCH 03/11] add test/comment for fix-instruction-processor addition --- src/common/components/fix-instruction-processor.tsx | 4 ++++ .../tests/injected/fix-instruction-processor.test.tsx | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/common/components/fix-instruction-processor.tsx b/src/common/components/fix-instruction-processor.tsx index 3f9d3222e2..348c3947b5 100644 --- a/src/common/components/fix-instruction-processor.tsx +++ b/src/common/components/fix-instruction-processor.tsx @@ -46,6 +46,10 @@ export class FixInstructionProcessor { private readonly originalMiddleSentence = ' and the original foreground color: '; public process(fixInstruction: string, recommendColor: RecommendColor): JSX.Element { + // We perform this replacement because what axe-core exposes as a "relatedNodes" property + // is presented in our cards views as a "Related paths" row. This only comes up in practice + // with the aria-required-children rule and is likely to be obsoleted with the resolution of + // https://github.com/dequelabs/axe-core/issues/3842 fixInstruction = fixInstruction.replace(/\(see related nodes\)/g, '(see related paths)'); const matches = this.getColorMatches(fixInstruction); diff --git a/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx b/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx index ea509b501f..f1bb7a3e03 100644 --- a/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx +++ b/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { FixInstructionProcessor } from 'common/components/fix-instruction-processor'; import { RecommendColor } from 'common/components/recommend-color'; +import * as React from 'react'; import { Mock, Times } from 'typemoq'; describe('FixInstructionProcessor', () => { @@ -13,6 +14,14 @@ describe('FixInstructionProcessor', () => { testSubject = new FixInstructionProcessor(); }); + test('updates aria-required-children "(see related nodes)" text to "(see related paths)"', () => { + const fixInstruction = 'Element has children which are not allowed (see related nodes)'; + + const result = testSubject.process(fixInstruction, recommendColorMock.object); + + expect(result).toEqual(<>Element has children which are not allowed (see related paths)); + }); + test('no background nor foreground on the message', () => { const fixInstruction = 'there is nothing that will trigger the processor to change the fix instruction here'; From 6489996dcb42ca00b2e0fe6f340bdd7e7dc04ba4 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Mon, 30 Jan 2023 15:27:58 -0500 Subject: [PATCH 04/11] add tests for related-paths-card-row --- .../cards/related-paths-card-row.tsx | 2 +- .../related-paths-card-row.test.tsx.snap | 23 +++++++++++++ .../cards/related-paths-card-row.test.tsx | 32 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 src/tests/unit/tests/common/components/cards/__snapshots__/related-paths-card-row.test.tsx.snap create mode 100644 src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx diff --git a/src/common/components/cards/related-paths-card-row.tsx b/src/common/components/cards/related-paths-card-row.tsx index d2303e055b..eec48a9ec0 100644 --- a/src/common/components/cards/related-paths-card-row.tsx +++ b/src/common/components/cards/related-paths-card-row.tsx @@ -12,7 +12,7 @@ export interface RelatedPathsCardRowProps extends CardRowProps { } export const RelatedPathsCardRow = NamedFC( - 'RichResolutionCardRow', + 'RelatedPathsCardRow', ({ index, propertyData }) => { if (isEmpty(propertyData)) { return null; diff --git a/src/tests/unit/tests/common/components/cards/__snapshots__/related-paths-card-row.test.tsx.snap b/src/tests/unit/tests/common/components/cards/__snapshots__/related-paths-card-row.test.tsx.snap new file mode 100644 index 0000000000..f98415d8b7 --- /dev/null +++ b/src/tests/unit/tests/common/components/cards/__snapshots__/related-paths-card-row.test.tsx.snap @@ -0,0 +1,23 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`RelatedPathsCardRow renders matching snapshot with related paths present 1`] = ` + +
  • + #path-1a;.path-1b +
    • +
  • + #path-2 +
    • +
  • + .path-3 +
    • + + } + label="Related paths" + rowKey="related-paths-row-123" +/> +`; diff --git a/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx b/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx new file mode 100644 index 0000000000..2d17821519 --- /dev/null +++ b/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { + RelatedPathsCardRow, + RelatedPathsCardRowProps, +} from 'common/components/cards/related-paths-card-row'; +import { shallow } from 'enzyme'; +import * as React from 'react'; + +describe(RelatedPathsCardRow.displayName, () => { + it.each([null, undefined, []])('renders as null with related paths: %p', relatedPaths => { + const props: RelatedPathsCardRowProps = { + deps: null, + index: 123, + propertyData: relatedPaths, + }; + const testSubject = shallow(); + + expect(testSubject.getElement()).toBeNull(); + }); + + it('renders matching snapshot with related paths present', () => { + const props: RelatedPathsCardRowProps = { + deps: null, + index: 123, + propertyData: ['#path-1a;.path-1b', '#path-2', '.path-3'], + }; + const testSubject = shallow(); + + expect(testSubject.getElement()).toMatchSnapshot(); + }); +}); From f52b1351ec025202eaaffe7b458d81b5d4c37f1e Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Mon, 30 Jan 2023 15:48:39 -0500 Subject: [PATCH 05/11] Use same selector normalization logic for combined report --- .../adapters/extract-related-selectors.ts | 34 +++++++------ .../package/accessibilityInsightsReport.d.ts | 3 +- ...mbined-results-to-cards-model-converter.ts | 4 ++ src/reports/package/reporter-factory.ts | 3 +- .../extract-related-selectors.test.ts | 48 ++++++++++++++++++- ...ults-to-cards-model-converter.test.ts.snap | 33 +++++++++++++ ...d-results-to-cards-model-converter.test.ts | 4 ++ 7 files changed, 112 insertions(+), 17 deletions(-) diff --git a/src/injected/adapters/extract-related-selectors.ts b/src/injected/adapters/extract-related-selectors.ts index 35638a97e6..a5eee6d12d 100644 --- a/src/injected/adapters/extract-related-selectors.ts +++ b/src/injected/adapters/extract-related-selectors.ts @@ -4,28 +4,34 @@ import { TargetHelper } from 'common/target-helper'; import { uniq } from 'lodash'; import { AxeNodeResult } from '../../scanner/iruleresults'; +export type RelatedSelectorNormalizer = typeof normalizeRelatedSelectors; export type RelatedSelectorExtractor = typeof extractRelatedSelectors; +export function normalizeRelatedSelectors(nodeSelector: string, relatedSelectors?: string[]) { + // This doesn't usually change anything; it would mainly affect if a rule had multiple + // checks that each returned overlapping sets of related nodes. Axe generally returns + // nodes in a reasonable order, so we want to use a method that preserves order here. + relatedSelectors = uniq(relatedSelectors); + + // This does have a practical impact; some axe checks (eg, color-contrast) will occasionally + // return the node itself as a "related node", and we'd rather avoid this. + relatedSelectors = relatedSelectors.filter(relatedSelector => relatedSelector !== nodeSelector); + + return relatedSelectors.length === 0 ? undefined : relatedSelectors; +} + export function extractRelatedSelectors(nodeResult: AxeNodeResult): string[] | undefined { - let output: string[] = []; + const nodeSelector = TargetHelper.getSelectorFromTarget(nodeResult.target); + + const relatedSelectors: string[] = []; for (const checkType of ['all', 'any', 'none'] as const) { const checks = nodeResult[checkType]; for (const check of checks) { - const relatedSelectors = + const relatedSelectorsForCheck = check.relatedNodes?.map(n => TargetHelper.getSelectorFromTarget(n.target)) ?? []; - output.push(...relatedSelectors); + relatedSelectors.push(...relatedSelectorsForCheck); } } - // This doesn't usually change anything; it would mainly affect if a rule had multiple - // checks that each returned overlapping sets of related nodes. Axe generally returns - // nodes in a reasonable order, so we want to use a method that preserves order here. - output = uniq(output); - - // This does have a practical impact; some axe checks (eg, color-contrast) will occasionally - // return the node itself as a "related node", and we'd rather avoid this. - const selfSelector = TargetHelper.getSelectorFromTarget(nodeResult.target); - output = output.filter(relatedSelector => relatedSelector !== selfSelector); - - return output.length === 0 ? undefined : output; + return normalizeRelatedSelectors(nodeSelector, relatedSelectors); } diff --git a/src/reports/package/accessibilityInsightsReport.d.ts b/src/reports/package/accessibilityInsightsReport.d.ts index 046cf926d6..f9a0492a65 100644 --- a/src/reports/package/accessibilityInsightsReport.d.ts +++ b/src/reports/package/accessibilityInsightsReport.d.ts @@ -86,7 +86,8 @@ declare namespace AccessibilityInsightsReport { elementSelector: string, snippet: string, fix: HowToFixData, - rule: AxeRuleData + rule: AxeRuleData, + relatedSelectors?: string[], }; export interface FailuresGroup { diff --git a/src/reports/package/combined-results-to-cards-model-converter.ts b/src/reports/package/combined-results-to-cards-model-converter.ts index 3e96dec5e3..efe747bac3 100644 --- a/src/reports/package/combined-results-to-cards-model-converter.ts +++ b/src/reports/package/combined-results-to-cards-model-converter.ts @@ -5,6 +5,7 @@ import { CardSelectionViewData } from "common/get-card-selection-view-data"; import { CardResult, CardRuleResult, CardRuleResultsByStatus, CardsViewModel } from "common/types/store-data/card-view-model"; import { GuidanceLink } from "common/types/store-data/guidance-links"; import { UUIDGenerator } from "common/uid-generator"; +import { RelatedSelectorNormalizer } from "injected/adapters/extract-related-selectors"; import { ResolutionCreator } from "injected/adapters/resolution-creator"; import { IssueFilingUrlStringUtils } from "issue-filing/common/issue-filing-url-string-utils"; import { isNil } from "lodash"; @@ -18,6 +19,7 @@ export class CombinedResultsToCardsModelConverter { private readonly uuidGenerator: UUIDGenerator, private readonly helpUrlGetter: HelpUrlGetter, private readonly getFixResolution: ResolutionCreator, + private readonly normalizeRelatedSelectors: RelatedSelectorNormalizer, ) {} public convertResults = ( @@ -74,6 +76,7 @@ export class CombinedResultsToCardsModelConverter { private getFailureCardResult = (failureData: FailureData): CardResult => { const rule = failureData.rule; const cssSelector = failureData.elementSelector; + const relatedCssSelectors = this.normalizeRelatedSelectors(cssSelector, failureData.relatedSelectors); const urls: any = {}; if (failureData.urls) { @@ -100,6 +103,7 @@ export class CombinedResultsToCardsModelConverter { }, descriptors: { snippet: failureData.snippet, + relatedCssSelectors, }, resolution: { howToFixSummary: failureData.fix.failureSummary, diff --git a/src/reports/package/reporter-factory.ts b/src/reports/package/reporter-factory.ts index 7007b350d0..5dac49e91c 100644 --- a/src/reports/package/reporter-factory.ts +++ b/src/reports/package/reporter-factory.ts @@ -7,7 +7,7 @@ import { getA11yInsightsWebRuleUrl } from 'common/configs/a11y-insights-rule-res import { CardSelectionViewData } from 'common/get-card-selection-view-data'; import { getCardViewData } from 'common/rule-based-view-model-provider'; import { generateUID } from 'common/uid-generator'; -import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; +import { extractRelatedSelectors, normalizeRelatedSelectors } from 'injected/adapters/extract-related-selectors'; import { getCheckResolution, getFixResolution } from 'injected/adapters/resolution-creator'; import { ConvertScanResultsToUnifiedResults } from 'injected/adapters/scan-results-to-unified-results'; import { convertScanResultsToUnifiedRules } from 'injected/adapters/scan-results-to-unified-rules'; @@ -185,6 +185,7 @@ const combinedResultsReportGenerator = (parameters: CombinedReportParameters) => generateUID, helpUrlGetter, getFixResolution, + normalizeRelatedSelectors, ); return new CombinedResultsReport(deps, parameters, toolData, resultsToCardsConverter); diff --git a/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts b/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts index d41d352b3c..0c4fb36abb 100644 --- a/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts +++ b/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts @@ -1,9 +1,55 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; +import { + extractRelatedSelectors, + normalizeRelatedSelectors, +} from 'injected/adapters/extract-related-selectors'; import { AxeNodeResult, FormattedCheckResult, Target } from 'scanner/iruleresults'; +describe(normalizeRelatedSelectors, () => { + it('returns undefined if there are no relatedNodes', () => { + const output = normalizeRelatedSelectors('#node', []); + expect(output).toBeUndefined(); + }); + + it('returns undefined if the only relatedNodes are the node itself', () => { + const output = normalizeRelatedSelectors('#node', ['#node', '#node']); + expect(output).toBeUndefined(); + }); + + it('no-ops for simple targets per relatedNode', () => { + const output = normalizeRelatedSelectors('#node', ['#related-1', '.related-2']); + expect(output).toStrictEqual(['#related-1', '.related-2']); + }); + + it('omits duplicates', () => { + const output = normalizeRelatedSelectors('#node', [ + '#related-1', + '#related-2', + '#related-2', + '#related-3', + '#related-3', + '#related-1', + ]); + expect(output).toStrictEqual(['#related-1', '#related-2', '#related-3']); + }); + + it.each([[[1, 2, 3]], [[3, 2, 1]], [[1, 3, 2]]])( + 'maintains original order', + (order: number[]) => { + const relatedSelectors = order.map(n => `#related-${n}`); + const output = normalizeRelatedSelectors('#node', relatedSelectors); + expect(output).toStrictEqual(relatedSelectors); + }, + ); + + it('omits the node itself', () => { + const output = normalizeRelatedSelectors('#node', ['#related-1', '#node', '#related-2']); + expect(output).toStrictEqual(['#related-1', '#related-2']); + }); +}); + describe(extractRelatedSelectors, () => { function checkResult(relatedTargets?: Target[]): FormattedCheckResult { const result: FormattedCheckResult = { diff --git a/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap b/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap index 585e9ecf57..b460c9028f 100644 --- a/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap +++ b/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap @@ -13,6 +13,9 @@ exports[`CombinedResultsToCardsModelConverter with a mixture of url specificatio "nodes": [ { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + ], "snippet": "
    snippet 1
    ", }, "highlightStatus": "unavailable", @@ -89,6 +92,9 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` "nodes": [ { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + ], "snippet": "
    snippet 1
    ", }, "highlightStatus": "unavailable", @@ -116,6 +122,9 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` }, { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-2, undefined", + ], "snippet": "
    snippet 2
    ", }, "highlightStatus": "unavailable", @@ -160,6 +169,9 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` "nodes": [ { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-2-selector-1, undefined", + ], "snippet": "
    snippet 1
    ", }, "highlightStatus": "unavailable", @@ -187,6 +199,9 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` }, { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-2-selector-2, undefined", + ], "snippet": "
    snippet 2
    ", }, "highlightStatus": "unavailable", @@ -263,6 +278,9 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` "nodes": [ { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + ], "snippet": "
    snippet 1
    ", }, "highlightStatus": "unavailable", @@ -290,6 +308,9 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` }, { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-2, undefined", + ], "snippet": "
    snippet 2
    ", }, "highlightStatus": "unavailable", @@ -334,6 +355,9 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` "nodes": [ { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-2-selector-1, undefined", + ], "snippet": "
    snippet 1
    ", }, "highlightStatus": "unavailable", @@ -361,6 +385,9 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` }, { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-2-selector-2, undefined", + ], "snippet": "
    snippet 2
    ", }, "highlightStatus": "unavailable", @@ -484,6 +511,9 @@ exports[`CombinedResultsToCardsModelConverter without passed or inapplicable rul "nodes": [ { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + ], "snippet": "
    snippet 1
    ", }, "highlightStatus": "unavailable", @@ -511,6 +541,9 @@ exports[`CombinedResultsToCardsModelConverter without passed or inapplicable rul }, { "descriptors": { + "relatedCssSelectors": [ + "normalizeRelatedSelectors output for .failed-rule-1-selector-2, undefined", + ], "snippet": "
    snippet 2
    ", }, "highlightStatus": "unavailable", diff --git a/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts b/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts index e76439a34d..51808575fe 100644 --- a/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts +++ b/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts @@ -24,6 +24,9 @@ describe(CombinedResultsToCardsModelConverter, () => { } } as HelpUrlGetter; let resolutionCreatorMock: IMock; + const normalizeRelatedSelectorsStub = (selector: string, relatedSelectors?: string[]) => { + return [`normalizeRelatedSelectors output for ${selector}, ${relatedSelectors}`]; + } let testSubject: CombinedResultsToCardsModelConverter; @@ -42,6 +45,7 @@ describe(CombinedResultsToCardsModelConverter, () => { uuidGeneratorMock.object, helpUrlGetterStub, resolutionCreatorMock.object, + normalizeRelatedSelectorsStub, ); }) From f1247ba896e38b69d9786b4f9af041bece170ba6 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Mon, 30 Jan 2023 18:56:05 -0500 Subject: [PATCH 06/11] fix unused import --- src/injected/adapters/scan-results-to-unified-results.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/injected/adapters/scan-results-to-unified-results.ts b/src/injected/adapters/scan-results-to-unified-results.ts index f64f6dbd68..4fbeb224a2 100644 --- a/src/injected/adapters/scan-results-to-unified-results.ts +++ b/src/injected/adapters/scan-results-to-unified-results.ts @@ -10,7 +10,7 @@ import { UnifiedResult, } from '../../common/types/store-data/unified-data-interface'; import { UUIDGenerator } from '../../common/uid-generator'; -import { AxeNodeResult, RuleResult, ScanResults, Target } from '../../scanner/iruleresults'; +import { AxeNodeResult, RuleResult, ScanResults } from '../../scanner/iruleresults'; import { IssueFilingUrlStringUtils } from './../../issue-filing/common/issue-filing-url-string-utils'; export type ConvertScanResultsToUnifiedResultsDelegate = ( From 6414de880997fb4675d6685ee5ce98c6ab926804 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Mon, 30 Jan 2023 18:59:04 -0500 Subject: [PATCH 07/11] fix type:check error in unit test --- .../cards/related-paths-card-row.test.tsx | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx b/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx index 2d17821519..5c927d2c78 100644 --- a/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx +++ b/src/tests/unit/tests/common/components/cards/related-paths-card-row.test.tsx @@ -8,16 +8,19 @@ import { shallow } from 'enzyme'; import * as React from 'react'; describe(RelatedPathsCardRow.displayName, () => { - it.each([null, undefined, []])('renders as null with related paths: %p', relatedPaths => { - const props: RelatedPathsCardRowProps = { - deps: null, - index: 123, - propertyData: relatedPaths, - }; - const testSubject = shallow(); + it.each([[], null, undefined])( + 'renders as null with related paths: %p', + (relatedPaths?: any) => { + const props: RelatedPathsCardRowProps = { + deps: null, + index: 123, + propertyData: relatedPaths, + }; + const testSubject = shallow(); - expect(testSubject.getElement()).toBeNull(); - }); + expect(testSubject.getElement()).toBeNull(); + }, + ); it('renders matching snapshot with related paths present', () => { const props: RelatedPathsCardRowProps = { From b67fa79fe8a230aaaac05c1a53decc1ef7f89537 Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Mon, 30 Jan 2023 19:28:47 -0500 Subject: [PATCH 08/11] Update report:e2e snapshots --- .../axe-results-with-issues.snap.html | 241 ++++++++++++++++++ .../axe-results-without-issues.snap.html | 6 + ...sults-with-baseline-aware-issues.snap.html | 6 + .../combined-results-with-issues.snap.html | 6 + .../combined-results-without-issues.snap.html | 6 + .../summary-scan-with-issues.snap.html | 6 + .../summary-scan-without-issues.snap.html | 6 + 7 files changed, 277 insertions(+) diff --git a/packages/report-e2e-tests/src/examples/axe-results-with-issues.snap.html b/packages/report-e2e-tests/src/examples/axe-results-with-issues.snap.html index b63fa2cc98..96899b3151 100644 --- a/packages/report-e2e-tests/src/examples/axe-results-with-issues.snap.html +++ b/packages/report-e2e-tests/src/examples/axe-results-with-issues.snap.html @@ -957,6 +957,12 @@ display: flex; } + /* css-modules:src/common/components/cards/related-paths-card-row */ + .path-list--MxxKM { + padding-left: 16px; + list-style-type: disc; + } + /* css-modules:src/common/components/cards/rich-resolution-content */ .multi-line-text-yes-bullet--TXyX- { padding-left: 16px; @@ -2113,6 +2119,14 @@ ><a href="#">About</a>Related paths
    • #menu > li:nth-child(1)
    How to fix<a href="#">Academics</a>Related paths
    • #menu > li:nth-child(2)
    How to fix<a href="#">Admissions</a>Related paths
    • #menu > li:nth-child(3)
    How to fix<a href="#">Visitors</a>Related paths
    • #menu > li:nth-child(4)
    How to fix<html class="deque-axe-is-ready">Related paths
    • #logo
    • li:nth-child(1) > + a[href="\#"]
    • li:nth-child(2) > + a[href="\#"]
    • li:nth-child(3) > + a[href="\#"]
    • li:nth-child(4) > + a[href="\#"]
    • img[src$="slide2\.jpg"]
    • a[href="somepage\.html\?ref\=Slide\%202"] + > .description
    • img[src$="arrow-left\.png"]
    • img[src$="arrow-right\.png"]
    • .heading:nth-child(2)
    • p:nth-child(3)
    • .hr[src$="line_gradient\.gif"][alt="horizontal\ + line\ graphic"]:nth-child(4)
    • .heading:nth-child(5)
    • p:nth-child(6)
    • .hr[src$="line_gradient\.gif"][alt="horizontal\ + line\ graphic"]:nth-child(7)
    • .heading:nth-child(8)
    • p:nth-child(9)
    • .hr[src$="line_gradient\.gif"][alt="horizontal\ + line\ graphic"]:nth-child(10)
    • .heading:nth-child(11)
    • td[colspan="\36 + "]:nth-child(2) > b
    • td[colspan="\36 + "]:nth-child(3) > b
    • tr:nth-child(2) > td:nth-child(2) + > b
    • tr:nth-child(2) > td:nth-child(3) + > b
    • td:nth-child(4) > b
    • td:nth-child(5) > b
    • td:nth-child(6) > b
    • td:nth-child(7) > b
    • td:nth-child(8) > b
    • td:nth-child(9) > b
    • td:nth-child(10) > b
    • td:nth-child(11) > b
    • td:nth-child(12) > b
    • td:nth-child(13) > b
    • td:nth-child(1) > b
    • tr:nth-child(3) > + td:nth-child(2)
    • tr:nth-child(3) > + td:nth-child(3)
    • tr:nth-child(3) > + td:nth-child(4)
    • tr:nth-child(3) > + td:nth-child(5)
    • tr:nth-child(3) > + td:nth-child(6)
    • tr:nth-child(3) > + td:nth-child(7)
    • tr:nth-child(3) > + td:nth-child(8)
    • tr:nth-child(3) > + td:nth-child(9)
    • tr:nth-child(3) > + td:nth-child(10)
    • tr:nth-child(3) > + td:nth-child(11)
    • tr:nth-child(3) > + td:nth-child(12)
    • tr:nth-child(3) > + td:nth-child(13)
    • tr:nth-child(4) > + td:nth-child(1)
    • tr:nth-child(4) > + td:nth-child(2)
    • tr:nth-child(4) > + td:nth-child(3)
    • tr:nth-child(4) > + td:nth-child(4)
    • tr:nth-child(4) > + td:nth-child(5)
    • tr:nth-child(4) > + td:nth-child(6)
    • tr:nth-child(4) > + td:nth-child(7)
    • tr:nth-child(4) > + td:nth-child(8)
    • tr:nth-child(4) > + td:nth-child(9)
    • tr:nth-child(4) > + td:nth-child(10)
    • tr:nth-child(4) > + td:nth-child(11)
    • tr:nth-child(4) > + td:nth-child(12)
    • tr:nth-child(4) > + td:nth-child(13)
    • tr:nth-child(5) > + td:nth-child(1)
    • tr:nth-child(5) > + td:nth-child(2)
    • tr:nth-child(5) > + td:nth-child(3)
    • tr:nth-child(5) > + td:nth-child(4)
    • tr:nth-child(5) > + td:nth-child(5)
    • tr:nth-child(5) > + td:nth-child(6)
    • tr:nth-child(5) > + td:nth-child(7)
    • tr:nth-child(5) > + td:nth-child(8)
    • tr:nth-child(5) > + td:nth-child(9)
    • tr:nth-child(5) > + td:nth-child(10)
    • tr:nth-child(5) > + td:nth-child(11)
    • tr:nth-child(5) > + td:nth-child(12)
    • tr:nth-child(5) > + td:nth-child(13)
    • #appForm > .heading
    • .reqExample
    • form > .required:nth-child(2)
    • .required:nth-child(3)
    • form > div:nth-child(4)
    • form > div:nth-child(5)
    • form > div:nth-child(6)
    • div:nth-child(7)
    • div:nth-child(8) > b
    • .major:nth-child(1)
    • .major:nth-child(2)
    • .major:nth-child(3)
    • .major:nth-child(4)
    • .major:nth-child(5)
    • .major:nth-child(6)
    • #captcha > b
    • #captcha > p
    • input[name="captcha"]
    • img[src$="captcha\.png"]
    • #submit
    • #footer
    How to fix Date: Mon, 30 Jan 2023 19:39:54 -0500 Subject: [PATCH 09/11] Update combined report path to use data already embedded in fix --- ...sults-with-baseline-aware-issues.snap.html | 10 ++++ .../combined-results-with-issues.snap.html | 10 ++++ .../visualization-scan-result-data.ts | 6 +++ .../adapters/extract-related-selectors.ts | 27 +++++------ .../package/accessibilityInsightsReport.d.ts | 1 - ...mbined-results-to-cards-model-converter.ts | 10 ++-- src/reports/package/reporter-factory.ts | 4 +- .../extract-related-selectors.test.ts | 48 +------------------ ...ults-to-cards-model-converter.test.ts.snap | 22 ++++----- ...d-results-to-cards-model-converter.test.ts | 6 +-- 10 files changed, 61 insertions(+), 83 deletions(-) diff --git a/packages/report-e2e-tests/src/examples/combined-results-with-baseline-aware-issues.snap.html b/packages/report-e2e-tests/src/examples/combined-results-with-baseline-aware-issues.snap.html index ea69d20d37..e93d6fbb3f 100644 --- a/packages/report-e2e-tests/src/examples/combined-results-with-baseline-aware-issues.snap.html +++ b/packages/report-e2e-tests/src/examples/combined-results-with-baseline-aware-issues.snap.html @@ -2405,6 +2405,16 @@ class="row-content--07KUh snippet--hSNfv" ><div>snippet</div>Related paths
    • #menu > li:nth-child(1)
    How to fix<div>snippet</div>Related paths
    • #menu > li:nth-child(1)
    How to fix relatedSelector !== nodeSelector); - - return relatedSelectors.length === 0 ? undefined : relatedSelectors; -} - export function extractRelatedSelectors(nodeResult: AxeNodeResult): string[] | undefined { const nodeSelector = TargetHelper.getSelectorFromTarget(nodeResult.target); - const relatedSelectors: string[] = []; + let relatedSelectors: string[] = []; for (const checkType of ['all', 'any', 'none'] as const) { const checks = nodeResult[checkType]; for (const check of checks) { @@ -33,5 +19,14 @@ export function extractRelatedSelectors(nodeResult: AxeNodeResult): string[] | u } } - return normalizeRelatedSelectors(nodeSelector, relatedSelectors); + // This doesn't usually change anything; it would mainly affect if a rule had multiple + // checks that each returned overlapping sets of related nodes. Axe generally returns + // nodes in a reasonable order, so we want to use a method that preserves order here. + relatedSelectors = uniq(relatedSelectors); + + // This does have a practical impact; some axe checks (eg, color-contrast) will occasionally + // return the node itself as a "related node", and we'd rather avoid this. + relatedSelectors = relatedSelectors.filter(relatedSelector => relatedSelector !== nodeSelector); + + return relatedSelectors.length === 0 ? undefined : relatedSelectors; } diff --git a/src/reports/package/accessibilityInsightsReport.d.ts b/src/reports/package/accessibilityInsightsReport.d.ts index f9a0492a65..136189cf54 100644 --- a/src/reports/package/accessibilityInsightsReport.d.ts +++ b/src/reports/package/accessibilityInsightsReport.d.ts @@ -87,7 +87,6 @@ declare namespace AccessibilityInsightsReport { snippet: string, fix: HowToFixData, rule: AxeRuleData, - relatedSelectors?: string[], }; export interface FailuresGroup { diff --git a/src/reports/package/combined-results-to-cards-model-converter.ts b/src/reports/package/combined-results-to-cards-model-converter.ts index efe747bac3..3c73b57150 100644 --- a/src/reports/package/combined-results-to-cards-model-converter.ts +++ b/src/reports/package/combined-results-to-cards-model-converter.ts @@ -5,7 +5,7 @@ import { CardSelectionViewData } from "common/get-card-selection-view-data"; import { CardResult, CardRuleResult, CardRuleResultsByStatus, CardsViewModel } from "common/types/store-data/card-view-model"; import { GuidanceLink } from "common/types/store-data/guidance-links"; import { UUIDGenerator } from "common/uid-generator"; -import { RelatedSelectorNormalizer } from "injected/adapters/extract-related-selectors"; +import { RelatedSelectorExtractor } from "injected/adapters/extract-related-selectors"; import { ResolutionCreator } from "injected/adapters/resolution-creator"; import { IssueFilingUrlStringUtils } from "issue-filing/common/issue-filing-url-string-utils"; import { isNil } from "lodash"; @@ -19,7 +19,7 @@ export class CombinedResultsToCardsModelConverter { private readonly uuidGenerator: UUIDGenerator, private readonly helpUrlGetter: HelpUrlGetter, private readonly getFixResolution: ResolutionCreator, - private readonly normalizeRelatedSelectors: RelatedSelectorNormalizer, + private readonly extractRelatedSelectors: RelatedSelectorExtractor, ) {} public convertResults = ( @@ -76,7 +76,11 @@ export class CombinedResultsToCardsModelConverter { private getFailureCardResult = (failureData: FailureData): CardResult => { const rule = failureData.rule; const cssSelector = failureData.elementSelector; - const relatedCssSelectors = this.normalizeRelatedSelectors(cssSelector, failureData.relatedSelectors); + const relatedCssSelectors = this.extractRelatedSelectors({ + target: [cssSelector], + html: failureData.snippet, + ...failureData.fix, + }); const urls: any = {}; if (failureData.urls) { diff --git a/src/reports/package/reporter-factory.ts b/src/reports/package/reporter-factory.ts index 5dac49e91c..b10e082f7c 100644 --- a/src/reports/package/reporter-factory.ts +++ b/src/reports/package/reporter-factory.ts @@ -7,7 +7,7 @@ import { getA11yInsightsWebRuleUrl } from 'common/configs/a11y-insights-rule-res import { CardSelectionViewData } from 'common/get-card-selection-view-data'; import { getCardViewData } from 'common/rule-based-view-model-provider'; import { generateUID } from 'common/uid-generator'; -import { extractRelatedSelectors, normalizeRelatedSelectors } from 'injected/adapters/extract-related-selectors'; +import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; import { getCheckResolution, getFixResolution } from 'injected/adapters/resolution-creator'; import { ConvertScanResultsToUnifiedResults } from 'injected/adapters/scan-results-to-unified-results'; import { convertScanResultsToUnifiedRules } from 'injected/adapters/scan-results-to-unified-rules'; @@ -185,7 +185,7 @@ const combinedResultsReportGenerator = (parameters: CombinedReportParameters) => generateUID, helpUrlGetter, getFixResolution, - normalizeRelatedSelectors, + extractRelatedSelectors, ); return new CombinedResultsReport(deps, parameters, toolData, resultsToCardsConverter); diff --git a/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts b/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts index 0c4fb36abb..d41d352b3c 100644 --- a/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts +++ b/src/tests/unit/tests/injected/adapters/extract-related-selectors.test.ts @@ -1,55 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { - extractRelatedSelectors, - normalizeRelatedSelectors, -} from 'injected/adapters/extract-related-selectors'; +import { extractRelatedSelectors } from 'injected/adapters/extract-related-selectors'; import { AxeNodeResult, FormattedCheckResult, Target } from 'scanner/iruleresults'; -describe(normalizeRelatedSelectors, () => { - it('returns undefined if there are no relatedNodes', () => { - const output = normalizeRelatedSelectors('#node', []); - expect(output).toBeUndefined(); - }); - - it('returns undefined if the only relatedNodes are the node itself', () => { - const output = normalizeRelatedSelectors('#node', ['#node', '#node']); - expect(output).toBeUndefined(); - }); - - it('no-ops for simple targets per relatedNode', () => { - const output = normalizeRelatedSelectors('#node', ['#related-1', '.related-2']); - expect(output).toStrictEqual(['#related-1', '.related-2']); - }); - - it('omits duplicates', () => { - const output = normalizeRelatedSelectors('#node', [ - '#related-1', - '#related-2', - '#related-2', - '#related-3', - '#related-3', - '#related-1', - ]); - expect(output).toStrictEqual(['#related-1', '#related-2', '#related-3']); - }); - - it.each([[[1, 2, 3]], [[3, 2, 1]], [[1, 3, 2]]])( - 'maintains original order', - (order: number[]) => { - const relatedSelectors = order.map(n => `#related-${n}`); - const output = normalizeRelatedSelectors('#node', relatedSelectors); - expect(output).toStrictEqual(relatedSelectors); - }, - ); - - it('omits the node itself', () => { - const output = normalizeRelatedSelectors('#node', ['#related-1', '#node', '#related-2']); - expect(output).toStrictEqual(['#related-1', '#related-2']); - }); -}); - describe(extractRelatedSelectors, () => { function checkResult(relatedTargets?: Target[]): FormattedCheckResult { const result: FormattedCheckResult = { diff --git a/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap b/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap index b460c9028f..21dbda3fb4 100644 --- a/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap +++ b/src/tests/unit/tests/reports/package/__snapshots__/combined-results-to-cards-model-converter.test.ts.snap @@ -14,7 +14,7 @@ exports[`CombinedResultsToCardsModelConverter with a mixture of url specificatio { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-1
    snippet 1
    ", ], "snippet": "
    snippet 1
    ", }, @@ -93,7 +93,7 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-1
    snippet 1
    ", ], "snippet": "
    snippet 1
    ", }, @@ -123,7 +123,7 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-2, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-2
    snippet 2
    ", ], "snippet": "
    snippet 2
    ", }, @@ -170,7 +170,7 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-2-selector-1, undefined", + "extractRelatedSelectors output for .failed-rule-2-selector-1
    snippet 1
    ", ], "snippet": "
    snippet 1
    ", }, @@ -200,7 +200,7 @@ exports[`CombinedResultsToCardsModelConverter with baseline-aware issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-2-selector-2, undefined", + "extractRelatedSelectors output for .failed-rule-2-selector-2
    snippet 2
    ", ], "snippet": "
    snippet 2
    ", }, @@ -279,7 +279,7 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-1
    snippet 1
    ", ], "snippet": "
    snippet 1
    ", }, @@ -309,7 +309,7 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-2, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-2
    snippet 2
    ", ], "snippet": "
    snippet 2
    ", }, @@ -356,7 +356,7 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-2-selector-1, undefined", + "extractRelatedSelectors output for .failed-rule-2-selector-1
    snippet 1
    ", ], "snippet": "
    snippet 1
    ", }, @@ -386,7 +386,7 @@ exports[`CombinedResultsToCardsModelConverter with issues 1`] = ` { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-2-selector-2, undefined", + "extractRelatedSelectors output for .failed-rule-2-selector-2
    snippet 2
    ", ], "snippet": "
    snippet 2
    ", }, @@ -512,7 +512,7 @@ exports[`CombinedResultsToCardsModelConverter without passed or inapplicable rul { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-1, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-1
    snippet 1
    ", ], "snippet": "
    snippet 1
    ", }, @@ -542,7 +542,7 @@ exports[`CombinedResultsToCardsModelConverter without passed or inapplicable rul { "descriptors": { "relatedCssSelectors": [ - "normalizeRelatedSelectors output for .failed-rule-1-selector-2, undefined", + "extractRelatedSelectors output for .failed-rule-1-selector-2
    snippet 2
    ", ], "snippet": "
    snippet 2
    ", }, diff --git a/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts b/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts index 51808575fe..7fff71e048 100644 --- a/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts +++ b/src/tests/unit/tests/reports/package/combined-results-to-cards-model-converter.test.ts @@ -24,8 +24,8 @@ describe(CombinedResultsToCardsModelConverter, () => { } } as HelpUrlGetter; let resolutionCreatorMock: IMock; - const normalizeRelatedSelectorsStub = (selector: string, relatedSelectors?: string[]) => { - return [`normalizeRelatedSelectors output for ${selector}, ${relatedSelectors}`]; + const extractRelatedSelectorsStub = (nodeResult: AxeNodeResult) => { + return [`extractRelatedSelectors output for ${nodeResult.target} ${nodeResult.html}`]; } let testSubject: CombinedResultsToCardsModelConverter; @@ -45,7 +45,7 @@ describe(CombinedResultsToCardsModelConverter, () => { uuidGeneratorMock.object, helpUrlGetterStub, resolutionCreatorMock.object, - normalizeRelatedSelectorsStub, + extractRelatedSelectorsStub, ); }) From b151f4beed2efc1fb25f504027f95b3c757c4d3e Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Tue, 31 Jan 2023 16:04:17 -0500 Subject: [PATCH 10/11] Update report typings to allow for addition of relatedNodes --- src/reports/package/accessibilityInsightsReport.d.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/reports/package/accessibilityInsightsReport.d.ts b/src/reports/package/accessibilityInsightsReport.d.ts index 136189cf54..41496ae637 100644 --- a/src/reports/package/accessibilityInsightsReport.d.ts +++ b/src/reports/package/accessibilityInsightsReport.d.ts @@ -65,6 +65,12 @@ declare namespace AccessibilityInsightsReport { id: string; message: string; data: any; + relatedNodes?: AxeRelatedNode[]; + } + + export interface AxeRelatedNode { + target: (string | string[])[]; + html: string; } export type HowToFixData = { From b6b1cb468a011074ebc1f82fc6b51c63ba73a35b Mon Sep 17 00:00:00 2001 From: Dan Bjorge Date: Tue, 31 Jan 2023 17:37:11 -0500 Subject: [PATCH 11/11] Emphasize "Related paths" in fix instruction per feedback from Fer --- .../components/fix-instruction-processor.tsx | 39 ++++++++++++++++--- .../fix-instruction-processor.test.tsx.snap | 17 +++++--- .../fix-instruction-processor.test.tsx | 8 ++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/common/components/fix-instruction-processor.tsx b/src/common/components/fix-instruction-processor.tsx index 348c3947b5..23fd875806 100644 --- a/src/common/components/fix-instruction-processor.tsx +++ b/src/common/components/fix-instruction-processor.tsx @@ -46,13 +46,42 @@ export class FixInstructionProcessor { private readonly originalMiddleSentence = ' and the original foreground color: '; public process(fixInstruction: string, recommendColor: RecommendColor): JSX.Element { - // We perform this replacement because what axe-core exposes as a "relatedNodes" property - // is presented in our cards views as a "Related paths" row. This only comes up in practice - // with the aria-required-children rule and is likely to be obsoleted with the resolution of - // https://github.com/dequelabs/axe-core/issues/3842 - fixInstruction = fixInstruction.replace(/\(see related nodes\)/g, '(see related paths)'); + return ( + this.tryProcessAsColorContrastRecommendation(fixInstruction, recommendColor) ?? + this.tryProcessAsRelatedNodesReference(fixInstruction) ?? + this.processAsNoop(fixInstruction) + ); + } + + private processAsNoop(fixInstruction: string): JSX.Element { + return <>{fixInstruction}; + } + // We perform this replacement because what axe-core exposes as a "relatedNodes" property + // is presented in our cards views as a "Related paths" row. This only comes up in practice + // with the aria-required-children rule and is likely to be obsoleted with the resolution of + // https://github.com/dequelabs/axe-core/issues/3842 + private tryProcessAsRelatedNodesReference(input: string): JSX.Element | null { + if (!input.endsWith(' (see related nodes)')) { + return null; + } + + const inputBody = input.slice(0, input.length - ' (see related nodes)'.length); + return ( + <> + {inputBody} (see Related paths) + + ); + } + + private tryProcessAsColorContrastRecommendation( + fixInstruction: string, + recommendColor: RecommendColor, + ): JSX.Element | null { const matches = this.getColorMatches(fixInstruction); + if (matches.length === 0) { + return null; + } let recommendationSentences: string[] = []; if (matches.length === 2 && fixInstruction != null) { diff --git a/src/tests/unit/tests/injected/__snapshots__/fix-instruction-processor.test.tsx.snap b/src/tests/unit/tests/injected/__snapshots__/fix-instruction-processor.test.tsx.snap index bfca5facb8..a6ab4736a0 100644 --- a/src/tests/unit/tests/injected/__snapshots__/fix-instruction-processor.test.tsx.snap +++ b/src/tests/unit/tests/injected/__snapshots__/fix-instruction-processor.test.tsx.snap @@ -157,12 +157,6 @@ exports[`FixInstructionProcessor foreground color on the message 1`] = ` `; -exports[`FixInstructionProcessor no background nor foreground on the message 1`] = ` - - there is nothing that will trigger the processor to change the fix instruction here - -`; - exports[`FixInstructionProcessor recommendColor has one recommendation 1`] = ` @@ -377,6 +371,17 @@ exports[`FixInstructionProcessor recommendColor has two recommendations 1`] = ` `; +exports[`FixInstructionProcessor updates aria-required-children "(see related nodes)" text to "(see Related paths)" 1`] = ` + + Element has children which are not allowed + (see + + Related paths + + ) + +`; + exports[`FixInstructionProcessor we assume only one instance of fore/background color, second instance will be ignored 1`] = ` diff --git a/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx b/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx index f1bb7a3e03..a10cc7988b 100644 --- a/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx +++ b/src/tests/unit/tests/injected/fix-instruction-processor.test.tsx @@ -14,21 +14,21 @@ describe('FixInstructionProcessor', () => { testSubject = new FixInstructionProcessor(); }); - test('updates aria-required-children "(see related nodes)" text to "(see related paths)"', () => { + test('updates aria-required-children "(see related nodes)" text to "(see Related paths)"', () => { const fixInstruction = 'Element has children which are not allowed (see related nodes)'; const result = testSubject.process(fixInstruction, recommendColorMock.object); - expect(result).toEqual(<>Element has children which are not allowed (see related paths)); + expect(result).toMatchSnapshot(); }); - test('no background nor foreground on the message', () => { + test('no-ops if neither background nor foreground on the message', () => { const fixInstruction = 'there is nothing that will trigger the processor to change the fix instruction here'; const result = testSubject.process(fixInstruction, recommendColorMock.object); - expect(result).toMatchSnapshot(); + expect(result).toEqual(<>{fixInstruction}); }); test('foreground color on the message', () => {