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

deps: axe-core@3.2.2 #8370

Merged
merged 3 commits into from
Apr 17, 2019
Merged

deps: axe-core@3.2.2 #8370

merged 3 commits into from
Apr 17, 2019

Conversation

paulirish
Copy link
Member

@paulirish paulirish commented Apr 17, 2019

has two perf boosts: https://github.com/dequelabs/axe-core/releases/tag/v3.2.0

3.2.0 introduced rules aria-hidden-focus and form-field-multiple-labels which are tagged wcag2a (which means we'll run them now).

The hidden-focus rule scares me a bit, implementation-wise, so I'm hesitant to include that in our results. The multiple-labels rule looks straightfoward.

@robdodson @marcysutton do yall have feelings on this?

fixes #7496
fixes #7355

@marcysutton
Copy link

Hey @paulirish! I don't work at Deque anymore, but I'm sure @WilcoFiers and @dylanb would love to know what reservations you have about the hidden-focus rule.

@robdodson
Copy link
Contributor

I think if we're unsure about the aria-hidden-focus rule we could disable it:
https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/gather/gatherers/accessibility.js#L40

@patrickhulce
Copy link
Collaborator

Shouldn't we disable all the rules we don't have audits for anyhow? :)

Yay for perf fixes!!! 🎉

@robdodson
Copy link
Contributor

@patrickhulce good point :)

@paulirish
Copy link
Member Author

paulirish commented Apr 17, 2019

Hey paulirish! I don't work at Deque anymore

oh shoot! i totally knew and forgot that. my bad!

dylan/wilco: mostly reservations as we're in a sprint towards tagging a new major version and i am scared of adding new audits to put in front of users that we haven't spent time with. After Google I/O we'll have more breathing room to evaluate these. ;)

Shouldn't we disable all the rules we don't have audits for anyhow? :)

for sure. brendan did the research into this back in #7127

Here's what I'm thinking:

  1. we land this version bump with these 2 new axe rules disabled
  2. i immediately followup this with another PR to disable the 5 rules brendan found.
  3. I add these two axe rules to that evaluate new axe audits for inclusion #7127 and after I/O we'll decide if we'll surface them to LH users.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

LGTM2

This is adding 10KB after gzip to the bundle, FYI (wish bundlesize did a comparison to parent build and alerted on these things)

@ConradSollitt
Copy link

I found this from issue #8435 after finding a hidden label with correct for="id" was not working on a site I created that uses a label with placeholder on a filter input:

https://awesome-web-react.js.org/

Seems that hidden associated labels are still showing as not linked in Lighthouse.

Posting the work around below in case someone else comes across the issue. Also, I added a similar comment on the issue.

/* Original version: */
label { display: none; }

/* Fix to pass Lighthouse: */
label { position: absolute; top:-1000px; left:-1000px; }

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.

Upgrade axe to 3.2.0 0 Accessebility
6 participants