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

fix: auto-pass main landmark requirements for no-landmark pages #2644

Merged
merged 24 commits into from
May 14, 2020
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
469427e
Text updates
dbjorge May 4, 2020
11cb71d
Update auto-pass text with feedback from Fer
dbjorge May 4, 2020
09a92d8
Merge branch 'master' into 2521-clarify-main-landmark-reqs
dbjorge May 7, 2020
cefbcc3
Add auto-pass-if-no-landmarks behavior
dbjorge May 8, 2020
466a46d
Merge branch 'master' into 2521-clarify-main-landmark-reqs
dbjorge May 8, 2020
6ae4ce6
Enable results which are not visualizable
dbjorge May 8, 2020
49ec65e
Remove obsolete custom rule
dbjorge May 8, 2020
f4df6c7
Refactor main role test
dbjorge May 8, 2020
74b72fd
Fix broken no-repeating-content
dbjorge May 8, 2020
f5a7d0e
Remove test for removed file
dbjorge May 8, 2020
e764440
Merge branch 'master' into 2521-clarify-main-landmark-reqs
dbjorge May 12, 2020
9ec3c4c
Rename local var in auto-pass logic
dbjorge May 12, 2020
f007d66
Fix placement of copyright header
dbjorge May 12, 2020
0989938
Fix missing var name in signature of new requirement property
dbjorge May 12, 2020
43df19d
Update AVET tests for new disabled behavior
dbjorge May 12, 2020
2e7fd89
Add tests for doesResultHaveMainRole
dbjorge May 12, 2020
71e488d
Update assessment-data-converter tests
dbjorge May 12, 2020
76c4a7d
Update assessment store tests to cover isVisualizationSupported
dbjorge May 12, 2020
9d37595
Add assessment-store test covering new getInitialManualTestStatus beh…
dbjorge May 12, 2020
3923763
Fix fastpass warnings
dbjorge May 12, 2020
7cac6fa
Revert test-assessment-provider changes (move to other PR)
dbjorge May 12, 2020
fcfc728
Add test case to cover missing path
dbjorge May 12, 2020
3025ab0
Refactor: improve formatting/naming
dbjorge May 12, 2020
14274d8
Add a comment to clarify answer to question from PR feedback
dbjorge May 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { GeneratedAssessmentInstance } from 'common/types/store-data/assessment-result-data';
import { isEmpty } from 'lodash';

import { BaseVisualHelperToggle } from './base-visual-helper-toggle';

export class AssessmentVisualizationEnabledToggle extends BaseVisualHelperToggle {
protected isDisabled(filteredInstances: GeneratedAssessmentInstance<{}, {}>[]): boolean {
return isEmpty(filteredInstances);
protected isDisabled(instances: GeneratedAssessmentInstance<{}, {}>[]): boolean {
return !this.isAnyInstanceVisualizable(instances);
}

protected isChecked(instances: GeneratedAssessmentInstance<{}, {}>[]): boolean {
Expand All @@ -28,10 +27,16 @@ export class AssessmentVisualizationEnabledToggle extends BaseVisualHelperToggle
};

private isAnyInstanceVisible(instances: GeneratedAssessmentInstance<{}, {}>[]): boolean {
const testStep = this.props.assessmentNavState.selectedTestSubview;
return instances.some(
instance =>
instance.testStepResults[this.props.assessmentNavState.selectedTestSubview]
.isVisualizationEnabled,
instance => instance.testStepResults[testStep].isVisualizationEnabled,
);
}

private isAnyInstanceVisualizable(instances: GeneratedAssessmentInstance<{}, {}>[]): boolean {
const testStep = this.props.assessmentNavState.selectedTestSubview;
return instances.some(
instance => instance.testStepResults[testStep].isVisualizationSupported,
);
}
}
20 changes: 20 additions & 0 deletions src/assessments/assessment-builder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { RequirementComparer } from 'common/assessment/requirement-comparer';
import { AssessmentVisualizationConfiguration } from 'common/configs/assessment-visualization-configuration';
import { Messages } from 'common/messages';
import { ManualTestStatus } from 'common/types/manual-test-status';
import { InstanceIdToInstanceDataMap } from 'common/types/store-data/assessment-result-data';
import { FeatureFlagStoreData } from 'common/types/store-data/feature-flag-store-data';
import { AssessmentScanData, ScanData } from 'common/types/store-data/visualization-store-data';
import {
Expand Down Expand Up @@ -52,6 +53,15 @@ export class AssessmentBuilder {
requirement.getInstanceStatus = AssessmentBuilder.getInstanceStatus;
}

if (!requirement.getInitialManualTestStatus) {
requirement.getInitialManualTestStatus = AssessmentBuilder.getInitialManualTestStatus;
}

if (!requirement.isVisualizationSupportedForResult) {
requirement.isVisualizationSupportedForResult =
AssessmentBuilder.isVisualizationSupportedForResult;
}

if (!requirement.getInstanceStatusColumns) {
requirement.getInstanceStatusColumns = AssessmentBuilder.getInstanceStatusColumns;
}
Expand All @@ -75,6 +85,16 @@ export class AssessmentBuilder {
return ManualTestStatus.UNKNOWN;
}

private static getInitialManualTestStatus(
instances: InstanceIdToInstanceDataMap,
): ManualTestStatus {
return ManualTestStatus.UNKNOWN;
}

private static isVisualizationSupportedForResult(result: DecoratedAxeNodeResult): boolean {
return true;
}

private static getInstanceStatusColumns(): Readonly<IColumn>[] {
return [
{
Expand Down
14 changes: 14 additions & 0 deletions src/assessments/landmarks/auto-pass-if-no-landmarks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { ManualTestStatus } from 'common/types/manual-test-status';
import { InstanceIdToInstanceDataMap } from 'common/types/store-data/assessment-result-data';
import { some } from 'lodash';

export function autoPassIfNoLandmarks(instanceData: InstanceIdToInstanceDataMap): ManualTestStatus {
const someInstanceHasLandmarkRole = some(
Object.values(instanceData),
instance => instance.propertyBag != null && instance.propertyBag['role'] != null,
);

return someInstanceHasLandmarkRole ? ManualTestStatus.UNKNOWN : ManualTestStatus.PASS;
}
11 changes: 11 additions & 0 deletions src/assessments/landmarks/does-result-have-main-role.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { DecoratedAxeNodeResult } from 'injected/scanner-utils';
import { some } from 'lodash';

export function doesResultHaveMainRole(result: DecoratedAxeNodeResult): boolean {
return (
some(result.any, checkResult => checkResult.data['role'] === 'main') ||
some(result.all, checkResult => checkResult.data['role'] === 'main')
);
}
38 changes: 34 additions & 4 deletions src/assessments/landmarks/test-steps/no-repeating-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.
import * as React from 'react';

import { doesResultHaveMainRole } from 'assessments/landmarks/does-result-have-main-role';
import { VisualizationType } from 'common/types/visualization-type';
import { link } from 'content/link';
import * as content from 'content/test/landmarks/no-repeating-content';
Expand All @@ -10,10 +11,14 @@ import { AnalyzerConfigurationFactory } from '../../common/analyzer-configuratio
import { ManualTestRecordYourResults } from '../../common/manual-test-record-your-results';
import * as Markup from '../../markup';
import { Requirement } from '../../types/requirement';
import { autoPassIfNoLandmarks } from '../auto-pass-if-no-landmarks';
import { LandmarkTestStep } from './test-steps';

const description: JSX.Element = (
<span>The main landmark must not contain any blocks of content that repeat across pages.</span>
<span>
The <Markup.CodeTerm>main</Markup.CodeTerm> landmark must not contain any blocks of content
that repeat across pages.
</span>
);

const howToTest: JSX.Element = (
Expand All @@ -22,10 +27,33 @@ const howToTest: JSX.Element = (
The visual helper for this requirement highlights the page's{' '}
<Markup.CodeTerm>main</Markup.CodeTerm> landmark.
</p>
<p>
<Markup.Emphasis>
Note: If no landmarks are found, this requirement will automatically be marked as
pass.
</Markup.Emphasis>
</p>
<ol>
<li>
In the target page, examine the main landmark to verify that it does not contain any
blocks of content that repeat across pages (e.g., site-wide navigation links).
<p>Examine the target page to verify that all of the following are true:</p>
<ol>
<li>
The page has exactly one <Markup.CodeTerm>main</Markup.CodeTerm> landmark,
and
</li>
<li>
The <Markup.CodeTerm>main</Markup.CodeTerm> landmark does not contain any
blocks of content that repeat across pages (such as site-wide navigation
links).
</li>
</ol>
<p>
Exception: If a page has nested <Markup.CodeTerm>document</Markup.CodeTerm> or{' '}
<Markup.CodeTerm>application</Markup.CodeTerm> roles (typically applied to{' '}
<Markup.Tag tagName="iframe" /> or <Markup.Tag tagName="frame" /> elements),
each nested document or application may <Markup.Emphasis>also</Markup.Emphasis>{' '}
have one <Markup.CodeTerm>main</Markup.CodeTerm> landmark.
</p>
</li>
<ManualTestRecordYourResults isMultipleFailurePossible={true} />
</ol>
Expand All @@ -38,11 +66,13 @@ export const NoRepeatingContent: Requirement = {
description,
howToTest,
isManual: true,
getInitialManualTestStatus: autoPassIfNoLandmarks,
isVisualizationSupportedForResult: doesResultHaveMainRole,
guidanceLinks: [link.WCAG_1_3_1, link.WCAG_2_4_1],
getAnalyzer: provider =>
provider.createRuleAnalyzer(
AnalyzerConfigurationFactory.forScanner({
rules: ['main-landmark'],
rules: ['unique-landmark'],
key: LandmarkTestStep.noRepeatingContent,
testType: VisualizationType.LandmarksAssessment,
}),
Expand Down
42 changes: 37 additions & 5 deletions src/assessments/landmarks/test-steps/primary-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.
import * as React from 'react';

import { doesResultHaveMainRole } from 'assessments/landmarks/does-result-have-main-role';
import { VisualizationType } from 'common/types/visualization-type';
import { link } from 'content/link';
import * as content from 'content/test/landmarks/primary-content';
Expand All @@ -10,19 +11,48 @@ import { AnalyzerConfigurationFactory } from '../../common/analyzer-configuratio
import { ManualTestRecordYourResults } from '../../common/manual-test-record-your-results';
import * as Markup from '../../markup';
import { Requirement } from '../../types/requirement';
import { autoPassIfNoLandmarks } from '../auto-pass-if-no-landmarks';
import { LandmarkTestStep } from './test-steps';

const description: JSX.Element = (
<span>The main landmark must contain all of the page's primary content.</span>
<span>
The <Markup.CodeTerm>main</Markup.CodeTerm> landmark must contain all of the page's primary
content.
</span>
);

const howToTest: JSX.Element = (
<div>
<p>The visual helper for this requirement highlights the page's main landmark.</p>
<p>
The visual helper for this requirement highlights the page's{' '}
<Markup.CodeTerm>main</Markup.CodeTerm> landmark.
</p>
<p>
<Markup.Emphasis>
Note: If no landmarks are found, this requirement will automatically be marked as
pass.
</Markup.Emphasis>
</p>
<ol>
<li>
In the target page, examine the <Markup.CodeTerm>main</Markup.CodeTerm> landmark to
verify that it contains all of the page's primary content.
<p>Examine the target page to verify that all of the following are true:</p>
<ol>
<li>
The page has exactly one <Markup.CodeTerm>main</Markup.CodeTerm> landmark,
and
</li>
<li>
The <Markup.CodeTerm>main</Markup.CodeTerm> landmark contains all of the
page's primary content.
</li>
</ol>
<p>
Exception: If a page has nested <Markup.CodeTerm>document</Markup.CodeTerm> or{' '}
karanbirsingh marked this conversation as resolved.
Show resolved Hide resolved
<Markup.CodeTerm>application</Markup.CodeTerm> roles (typically applied to{' '}
<Markup.Tag tagName="iframe" /> or <Markup.Tag tagName="frame" /> elements),
each nested document or application may <Markup.Emphasis>also</Markup.Emphasis>{' '}
have one <Markup.CodeTerm>main</Markup.CodeTerm> landmark.
</p>
</li>
<ManualTestRecordYourResults isMultipleFailurePossible={true} />
</ol>
Expand All @@ -35,11 +65,13 @@ export const PrimaryContent: Requirement = {
description,
howToTest,
isManual: true,
getInitialManualTestStatus: autoPassIfNoLandmarks,
isVisualizationSupportedForResult: doesResultHaveMainRole,
guidanceLinks: [link.WCAG_1_3_1, link.WCAG_2_4_1],
getAnalyzer: provider =>
provider.createRuleAnalyzer(
AnalyzerConfigurationFactory.forScanner({
rules: ['main-landmark'],
rules: ['unique-landmark'],
key: LandmarkTestStep.primaryContent,
testType: VisualizationType.LandmarksAssessment,
}),
Expand Down
8 changes: 8 additions & 0 deletions src/assessments/types/requirement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ManualTestStatus } from 'common/types/manual-test-status';
import {
AssessmentNavState,
GeneratedAssessmentInstance,
InstanceIdToInstanceDataMap,
} from 'common/types/store-data/assessment-result-data';
import { FeatureFlagStoreData } from 'common/types/store-data/feature-flag-store-data';
import { DetailsViewActionMessageCreator } from 'DetailsView/actions/details-view-action-message-creator';
Expand Down Expand Up @@ -39,10 +40,17 @@ export interface Requirement {
addFailureInstruction?: string;
infoAndExamples?: ContentPageComponent;
isManual: boolean;
// This is for semi-manual cases where we can't present a list of instances like an assisted
// test would, but can infer a PASS or FAIL state. If not specified, acts like () => UNKNOWN.
getInitialManualTestStatus?: (instances: InstanceIdToInstanceDataMap) => ManualTestStatus;
guidanceLinks: HyperlinkDefinition[];
columnsConfig?: InstanceTableColumn[];
getAnalyzer?: (provider: AnalyzerProvider) => Analyzer;
getVisualHelperToggle?: (props: VisualHelperToggleConfig) => JSX.Element;
// Any results this returns false for will be omitted from visual helper displays, but still
// present for the purposes of instance lists or getInitialManualTestStatus. By default, all
// results support visualization.
isVisualizationSupportedForResult?: (result: DecoratedAxeNodeResult) => boolean;
visualizationInstanceProcessor?: VisualizationInstanceProcessorCallback<
PropertyBags,
PropertyBags
Expand Down
7 changes: 7 additions & 0 deletions src/background/assessment-data-converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export class AssessmentDataConverter {
stepName: string,
generateInstanceIdentifier: (instance: UniquelyIdentifiableInstances) => string,
getInstanceStatus: (result: DecoratedAxeNodeResult) => ManualTestStatus,
isVisualizationSupported: (result: DecoratedAxeNodeResult) => boolean,
): AssessmentInstancesMap {
let instancesMap: AssessmentInstancesMap = {};

Expand All @@ -51,6 +52,7 @@ export class AssessmentDataConverter {
stepName,
ruleResult,
getInstanceStatus,
isVisualizationSupported,
);
}
});
Expand Down Expand Up @@ -98,6 +100,7 @@ export class AssessmentDataConverter {
testStep: string,
ruleResult: DecoratedAxeNodeResult,
getInstanceStatus: (result: DecoratedAxeNodeResult) => ManualTestStatus,
isVisualizationSupported: (result: DecoratedAxeNodeResult) => boolean,
): GeneratedAssessmentInstance {
const target: string[] = elementAxeResult.target;
let testStepResults = {};
Expand All @@ -114,6 +117,7 @@ export class AssessmentDataConverter {
ruleResult,
elementAxeResult,
getInstanceStatus,
isVisualizationSupported,
);

let actualPropertyBag = {
Expand Down Expand Up @@ -163,6 +167,7 @@ export class AssessmentDataConverter {
status: ManualTestStatus.UNKNOWN,
isCapturedByUser: false,
failureSummary: null,
isVisualizationSupported: true,
isVisualizationEnabled: true,
isVisible: true,
};
Expand All @@ -172,12 +177,14 @@ export class AssessmentDataConverter {
ruleResult: DecoratedAxeNodeResult,
elementAxeResult: HtmlElementAxeResults,
getInstanceStatus: (result: DecoratedAxeNodeResult) => ManualTestStatus,
isVisualizationSupported: (result: DecoratedAxeNodeResult) => boolean,
): TestStepResult {
return {
id: ruleResult.id,
status: getInstanceStatus(ruleResult),
isCapturedByUser: false,
failureSummary: ruleResult.failureSummary,
isVisualizationSupported: isVisualizationSupported(ruleResult),
isVisualizationEnabled: false,
isVisible: true,
};
Expand Down
Loading