-
Notifications
You must be signed in to change notification settings - Fork 68
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(playwright): Upgrade to axe-core@4.3.2 #334
Conversation
@@ -114,22 +123,32 @@ export default class AxeBuilder { | |||
*/ | |||
|
|||
public async analyze(): Promise<AxeResults> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this take a callback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do need to change it, it should be in a separate PR. Don't let that hold this up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Do we need Wilco to approve his one comment? https://github.com/dequelabs/axe-core-npm/pull/334/files#r690399615
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, forgot I had a change marked
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
blocked by: #335