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

feat(axe-core 4.8): Axe core version upgrade for Web #7274

Merged
merged 14 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
"@microsoft/applicationinsights-web": "^2.8.15",
"@testing-library/user-event": "^14.5.2",
"ajv": "^8.12.0",
"axe-core": "4.7.2",
"axe-core": "4.8.4",
"classnames": "^2.5.1",
"idb-keyval": "^6.2.1",
"lodash": "^4.17.21",
Expand Down
2 changes: 1 addition & 1 deletion packages/report/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
},
"dependencies": {
"@fluentui/react": "^8.96.1",
"axe-core": "4.7.2",
"axe-core": "4.8.4",
"classnames": "^2.5.1",
"lodash": "^4.17.21",
"luxon": "^3.4.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"dependencies": {
"@fluentui/react": "^8.96.1",
"axe-core": "4.7.2",
"axe-core": "4.8.4",
"classnames": "^2.5.1",
"lodash": "^4.17.21",
"luxon": "^3.4.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class LoadAssessmentDataSchemaProvider {
const deprecatedRequirements = [
{ assessmentKey: 'automated-checks', requirementKey: 'aria-roledescription' },
{ assessmentKey: 'automated-checks', requirementKey: 'duplicate-id' },
{ assessmentKey: 'automated-checks', requirementKey: 'duplicate-id-active' },
];
deprecatedRequirements.forEach(requirement => {
if (this.getAssessments(schema)[requirement.assessmentKey] === undefined) {
Expand Down
18 changes: 17 additions & 1 deletion src/scanner/axe-extension.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ declare module 'axe-core/axe' {
};
dom: {
isVisible: Function;
idrefs: (node: HTMLElement, attr: string) => HTMLElement[];
};
text: {
accessibleText: Function;
Expand All @@ -31,6 +30,23 @@ declare module 'axe-core/axe' {
// this must be surrounded by axe.setup and axe.teardown calls
getSelector: (element: HTMLElement) => string;
}
interface Dom {
isVisible: Function;
idrefs: (node: HTMLElement, attr: string) => HTMLElement[];
}
interface Aria {
label: Function;
implicitRole: Function;
getRolesByType: Function;
lookupTable: any;
}

interface Text {
accessibleText: Function;
isHumanInterpretable: Function;
sanitize: Function;
subtreeText: Function;
}
Comment on lines +33 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What impact do these changes have? Was there some kind of error without the interfaces? It seems odd to me that the calls that use these functions didn't need to be updated but I'm not that familiar with setting up a .d.ts file for a library like we've done here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @madalynrose
Yes correct, we will get errors.
Because for 4.7.2 the above properties were part of axe-extension.d.ts, because in 4.7.2 package these properties were not there. So its added in the code repo.
But in 4.8.4, these properties are added in package, but not fully like few properties are not added fully which is used by our code. So those missing properties are added above.
Please let me know if need further information.

Thanks


const _audit: {
rules: IRuleConfiguration[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ exports[`Popup -> Launch Pad content should match snapshot when quick assess fea
Navigate to axe-core npm page
</div>
</div>
4.7.2
4.8.4
</div>
</div>
</div>
Expand Down Expand Up @@ -587,7 +587,7 @@ exports[`Popup -> Launch Pad content should match snapshot when quick assess fea
Navigate to axe-core npm page
</div>
</div>
4.7.2
4.8.4
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ describe(LoadAssessmentDataSchemaProvider, () => {
? {
'aria-roledescription': stepProperties,
'duplicate-id': stepProperties,
'duplicate-id-active': stepProperties,
}
: {},
type: ['object', 'null'],
Expand All @@ -80,6 +81,7 @@ describe(LoadAssessmentDataSchemaProvider, () => {
? {
'aria-roledescription': statusProperties,
'duplicate-id': statusProperties,
'duplicate-id-active': statusProperties,
}
: {},
type: ['object', 'null'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,18 @@ exports[`getRuleInclusions getRuleInclusions matches snapshotted list of product
"reason": "best practice rule that was investigated with no known false positives, implemented as an automated check.",
"status": "included",
},
"aria-braille-equivalent": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling new rules usually includes adding documentation to info-examples. Have we done that for these?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PM team is working on the documentation for these. We will release both Web extension and docs in sync. @nang4ally to add more details if required.

"status": "included",
},
"aria-command-name": {
"status": "included",
},
"aria-conditional-attr": {
"status": "included",
},
"aria-deprecated-role": {
"status": "included",
},
"aria-dialog-name": {
"reason": "rule is tagged best-practice",
"status": "excluded",
Expand All @@ -38,6 +47,9 @@ exports[`getRuleInclusions getRuleInclusions matches snapshotted list of product
"aria-progressbar-name": {
"status": "included",
},
"aria-prohibited-attr": {
"status": "included",
},
"aria-required-attr": {
"status": "included",
},
Expand Down Expand Up @@ -118,7 +130,8 @@ exports[`getRuleInclusions getRuleInclusions matches snapshotted list of product
"status": "excluded",
},
"duplicate-id-active": {
"status": "included",
"reason": "disabled in axe config",
"status": "excluded",
},
"duplicate-id-aria": {
"status": "included",
Expand Down
6 changes: 3 additions & 3 deletions src/tests/unit/tests/scanner/accessible-text.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('axe.commons.text.accessibleText examples', () => {

const element1 = fixture.querySelector('#el1');

const accessibleText = withAxeSetup(() => Axe.commons.text.accessibleText(element1, false));
const accessibleText = withAxeSetup(() => Axe.commons.text.accessibleText(element1));
expect(accessibleText).toBe('hello');
});

Expand All @@ -32,7 +32,7 @@ describe('axe.commons.text.accessibleText examples', () => {

const element2 = fixture.querySelector('#el2');

const accessibleText = withAxeSetup(() => Axe.commons.text.accessibleText(element2, false));
const accessibleText = withAxeSetup(() => Axe.commons.text.accessibleText(element2));
expect(accessibleText).toBe('hello');
});

Expand All @@ -44,7 +44,7 @@ describe('axe.commons.text.accessibleText examples', () => {

const span = fixture.querySelector('#del_row2');

const accessibleText = withAxeSetup(() => Axe.commons.text.accessibleText(span, false));
const accessibleText = withAxeSetup(() => Axe.commons.text.accessibleText(span));
expect(accessibleText).toBe('Delete HolidayLetter.pdf');
});

Expand Down
14 changes: 7 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3283,7 +3283,7 @@ __metadata:
resolution: "accessibility-insights-report@workspace:packages/report"
dependencies:
"@fluentui/react": ^8.96.1
axe-core: 4.7.2
axe-core: 4.8.4
classnames: ^2.5.1
lodash: ^4.17.21
luxon: ^3.4.4
Expand All @@ -3299,7 +3299,7 @@ __metadata:
resolution: "accessibility-insights-ui@workspace:packages/ui"
dependencies:
"@fluentui/react": ^8.96.1
axe-core: 4.7.2
axe-core: 4.8.4
classnames: ^2.5.1
lodash: ^4.17.21
luxon: ^3.4.4
Expand Down Expand Up @@ -3340,7 +3340,7 @@ __metadata:
"@typescript-eslint/eslint-plugin": ^5.61.0
"@typescript-eslint/parser": ^6.18.1
ajv: ^8.12.0
axe-core: 4.7.2
axe-core: 4.8.4
case-sensitive-paths-webpack-plugin: ^2.4.0
classnames: ^2.5.1
codecov: ^3.8.3
Expand Down Expand Up @@ -3932,10 +3932,10 @@ __metadata:
languageName: node
linkType: hard

"axe-core@npm:4.7.2":
version: 4.7.2
resolution: "axe-core@npm:4.7.2"
checksum: 5d86fa0f45213b0e54cbb5d713ce885c4a8fe3a72b92dd915a47aa396d6fd149c4a87fec53aa978511f6d941402256cfeb26f2db35129e370f25a453c688655a
"axe-core@npm:4.8.4":
version: 4.8.4
resolution: "axe-core@npm:4.8.4"
checksum: 644da2fec17bcf6f834edaab1baa5a75a9a3ee370c323215c73da6d7e45fac11d01470d92d0a3d5f26695e15c1d9b781733dfbe1fe09d505076c58f09ed74e02
languageName: node
linkType: hard

Expand Down
Loading