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

Conversation

SaanicaG
Copy link
Contributor

@SaanicaG SaanicaG commented Mar 14, 2024

Details

This PR updates axe-core to its latest version, 4.8.4, from 4.7.2. It also bumps the accessibility-insights-report package version. As part of the axe-core update:

image
Motivation

This change is part of https://dev.azure.com/mseng/1ES/_workitems/edit/2157659/

Context

Pull request checklist

  • Addresses an existing issue: #0000
  • Ran yarn fastpass
  • Added/updated relevant unit test(s) (and ran yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • PR title AND final merge commit title both start with a semantic tag (fix:, chore:, feat(feature-name):, refactor:). See CONTRIBUTING.md.
  • (UI changes only) Added screenshots/GIFs to description above
  • (UI changes only) Verified usability with NVDA/JAWS

@v-viyada v-viyada force-pushed the v-sghate/AxeVersionUpgradeForWeb branch from 92d6f0b to ee00852 Compare March 21, 2024 14:29
@v-viyada v-viyada marked this pull request as ready for review March 22, 2024 22:46
@v-viyada v-viyada requested a review from a team as a code owner March 22, 2024 22:46
@v-viyada v-viyada marked this pull request as draft March 25, 2024 16:07
@v-rakeshsh v-rakeshsh force-pushed the v-sghate/AxeVersionUpgradeForWeb branch from 4a6d569 to eceb58c Compare March 25, 2024 17:00
@v-viyada v-viyada marked this pull request as ready for review March 25, 2024 17:01
Copy link
Contributor

@madalynrose madalynrose left a comment

Choose a reason for hiding this comment

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

The upgrade itself looks solid but I have a couple questions about implementation.

Comment on lines +33 to +49
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;
}
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

@@ -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.

@v-viyada v-viyada changed the title Axe core version upgrade for Web feat(axe-core 4.8): Axe core version upgrade for Web Apr 5, 2024
@v-viyada v-viyada merged commit df756f3 into microsoft:main Apr 25, 2024
12 of 13 checks passed
v-viyada added a commit that referenced this pull request Apr 26, 2024
#### Details

Update package version for accessibility insights report for axe-core
release. Please refer #7274

##### Motivation

<!-- This can be as simple as "addresses issue #123" -->

##### 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 -->
- [n/a] Addresses an existing issue: #0000
- [x] Ran `yarn fastpass`
- [n/a] 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`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants