Skip to content

Commit 4628163

Browse files
feat(implement-new-checks): enable aria-allowed-role, presentation-role-conflict, and p-as-heading (#6030)
#### Details This PR adds: - `aria-allowed-role` and `presentation-role-conflict` as automated check rules <img width="1004" alt="aria-allowed-role and presentation-role-conflict in FastPass' automated checks" src="https://user-images.githubusercontent.com/18401275/190701565-78ee4e28-cf75-4884-bdfb-1bac395de8bb.png"> - `p-as-heading` as a needs review rule <img width="1208" alt="p-as-heading in FastPass' Needs Review" src="https://user-images.githubusercontent.com/18401275/190701351-e58e9a3c-57cf-48d0-84d2-3167a53eaa5d.png"> ##### Motivation Feature 1981768 🪄 ##### Context <!-- Are there any parts that you've intentionally left out-of-scope for a later PR to handle? --> <!-- Were there any alternative approaches you considered? What tradeoffs did you consider? --> #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: 1986752 - [x] Ran `yarn null:autoadd` - [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 to description above - [ ] (UI changes only) Verified usability with NVDA/JAWS
1 parent 77cbb92 commit 4628163

File tree

8 files changed

+34
-5
lines changed

8 files changed

+34
-5
lines changed

src/ad-hoc-visualizations/needs-review/visualization.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const needsReviewRuleAnalyzerConfiguration: RuleAnalyzerConfiguration = {
2323
'link-in-text-block',
2424
'scrollable-region-focusable',
2525
'label-content-name-mismatch',
26+
'p-as-heading',
2627
],
2728
resultProcessor: (scanner: ScannerUtils) => scanner.getFailingInstances,
2829
telemetryProcessor: (telemetryFactory: TelemetryDataFactory) =>

src/common/components/cards/rich-resolution-content.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,15 @@ export const RichResolutionContent = NamedFC<RichResolutionContentProps>(
318318
</span>
319319
);
320320
}
321+
case 'web/p-as-heading': {
322+
return (
323+
<span>
324+
Inspect the {`<p>`} element and verify that the element is not used as a
325+
heading through visual styling with bold, italic text or font-size. If
326+
headings are needed, use the appropriate heading tags.
327+
</span>
328+
);
329+
}
321330
default: {
322331
throw new Error(
323332
`Cannot render RichResolutionContent with unrecognized contentId ${contentId}`,

src/scanner/get-rule-inclusions.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ export const explicitRuleOverrides: DictionaryStringTo<RuleIncluded> = {
2828
status: 'excluded',
2929
reason: 'only reports to needs-review results due to potential false-positives',
3030
},
31+
'aria-allowed-role': {
32+
status: 'included',
33+
reason: 'best practice rule that was investigated with no known false positives, implemented as an automated check.',
34+
},
35+
'presentation-role-conflict': {
36+
status: 'included',
37+
reason: 'best practice rule that was investigated with no known false positives, implemented as an automated check.',
38+
},
3139
};
3240

3341
export const getRuleInclusions = (

src/tests/end-to-end/tests/details-view/cross-origin-iframe.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,12 @@ describe('scanning', () => {
7676
fastPassAutomatedChecksSelectors.ruleDetail,
7777
);
7878

79-
expect(ruleDetails).toHaveLength(4);
79+
expect(ruleDetails).toHaveLength(5);
8080

8181
const expectedCounts = {
8282
'frame-title': 2,
8383
'html-has-lang': 1,
84+
'aria-allowed-role': 2,
8485
'image-alt': 9,
8586
label: 3,
8687
};

src/tests/unit/tests/common/components/cards/__snapshots__/rich-resolution-content.test.tsx.snap

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,14 @@ exports[`RichResolutionContent renders static content with id=web/link-in-text-b
398398
</ul>
399399
`;
400400

401+
exports[`RichResolutionContent renders static content with id=web/p-as-heading 1`] = `
402+
<span>
403+
Inspect the
404+
&lt;p&gt;
405+
element and verify that the element is not used as a heading through visual styling with bold, italic text or font-size. If headings are needed, use the appropriate heading tags.
406+
</span>
407+
`;
408+
401409
exports[`RichResolutionContent renders static content with id=web/scrollable-region-focusable 1`] = `
402410
<span>
403411
Examine the element and ensure that, if there is scrollable content, the elements are accessible by keyboard.

src/tests/unit/tests/common/components/cards/rich-resolution-content.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('RichResolutionContent', () => {
2626
'web/link-in-text-block',
2727
'web/scrollable-region-focusable',
2828
'web/label-content-name-mismatch',
29+
'web/p-as-heading',
2930
'android/atfa/ClassNameCheck',
3031
'android/atfa/ClickableSpanCheck',
3132
'android/atfa/DuplicateClickableBoundsCheck',

src/tests/unit/tests/common/self-fast-pass.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ describe('SelfFastPass', () => {
7474
'link-in-text-block',
7575
'scrollable-region-focusable',
7676
'label-content-name-mismatch',
77+
'p-as-heading',
7778
],
7879
},
7980
It.isAny(),

src/tests/unit/tests/scanner/__snapshots__/get-rule-inclusions.test.ts.snap

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ exports[`getRuleInclusions matches snapshotted list of production rules 1`] = `
1313
"status": "included",
1414
},
1515
"aria-allowed-role": {
16-
"reason": "rule is tagged best-practice",
17-
"status": "excluded",
16+
"reason": "best practice rule that was investigated with no known false positives, implemented as an automated check.",
17+
"status": "included",
1818
},
1919
"aria-command-name": {
2020
"status": "included",
@@ -279,8 +279,8 @@ exports[`getRuleInclusions matches snapshotted list of production rules 1`] = `
279279
"status": "excluded",
280280
},
281281
"presentation-role-conflict": {
282-
"reason": "rule is tagged best-practice",
283-
"status": "excluded",
282+
"reason": "best practice rule that was investigated with no known false positives, implemented as an automated check.",
283+
"status": "included",
284284
},
285285
"region": {
286286
"reason": "rule is tagged best-practice",

0 commit comments

Comments
 (0)