equalizedigital / accessibility-checker

GNU General Public License v2.0
15 stars 8 forks source link

Check possible_heading rule with computedStyles #622

Closed pattonwebz closed 1 month ago

pattonwebz commented 2 months ago

This PR moves the possible_heading rule to a JS rule that can check element styling with computedStyles. That prevents any need to parse CSS vars, functions or pseudo-classes since it gets the actual styles the element has after all the cascade is applied.

@amberhinds I will need some help setting the tags that the rule should have.

Sidenote: I like to see PRs that make things more powerful yet are red than green (or orange and blue in my case haha) :) Screenshot from 2024-05-14 22-18-28

Fixes: #612

amberhinds commented 2 months ago

@pattonwebz what do you mean by tags?

pattonwebz commented 2 months ago

@amberhinds sorry I should have linked to the list. I need to know the correct tags and a category from the list here: https://www.deque.com/axe/core-documentation/api-documentation/#axecore-tags

I presume this is the list but let me know if it misses anything or something shuoldn't be in the list:

wcag2a wcag2aa wcag21a wcag21aa wcag22aa wcag131
wcag241 wcag246

amberhinds commented 2 months ago

Got it. Here are the tags:

My understanding of "Each rule has one tag that indicates which WCAG version / level it belongs to" is that we don't need wcag2aa, wcag2aaa, etc. because if you were to query for AAA you would just query all three (wcag2a, wcag2aa, and wcag2aaa). Likewise, 2.2 includes 2.1 and 2.0; we want the tag to more accurately reflect what version and level the specific rule is.

amberhinds commented 2 months ago

@pattonwebz did you use this? https://dequeuniversity.com/rules/axe/4.9/p-as-heading?application=RuleDescription

pattonwebz commented 2 months ago

@pattonwebz did you use this? dequeuniversity.com/rules/axe/4.9/p-as-heading?application=RuleDescription

I didn't use that rule because it doesn't work as well as what we did in PHP. I replicated our own rule instead of using this.

The rule from axe-core doesn't check as many things as we do (it doesn't care about font size), and in my testing, the things it found were marked as incomplete, which means it needs additional review to determine a pass or fail. It can't tell anything with certainty from the data I have run it against — probably why it's still flagged as an experimental rule.

Screenshot from 2024-05-15 14-56-09

Our rule catches 8 items, and the axe-core rule catches 0 and returns a 'can't tell' result with only one item from my test data.

amberhinds commented 2 months ago

Thanks for that info! Sounds like our rule is better. I wonder if we need to document anywhere when our rule is better than an axe rule so we remember that for the future.

pattonwebz commented 2 months ago

I can add some comments in the code for this rule so that we can remember the next time we touch the code. I'll see if I can find a place to document it outside of the code for ourselves to and form the basis of some copy for the plugin docs that explains why our check does certain things that axe-core misses.

amberhinds commented 2 months ago

Code comments sound good. Outside the code tracking here makes sense: https://docs.google.com/spreadsheets/d/10vbbz4Y4uit76z73U8ffa9h6NN_ha5TC6Vfkt2-4IxI/edit?usp=sharing