-
Notifications
You must be signed in to change notification settings - Fork 779
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
fix(docs/rules): add missing category tags to rules metadata #2569
fix(docs/rules): add missing category tags to rules metadata #2569
Conversation
Add category tags to the rules that were missing them. Add validation to check during a build for the inclusion of a category on the rule metadata. Closes issue #2554
@@ -2,7 +2,7 @@ | |||
"id": "accesskeys", | |||
"selector": "[accesskey]", | |||
"excludeHidden": false, | |||
"tags": ["best-practice", "cat.keyboard"], | |||
"tags": ["cat.keyboard", "best-practice"], |
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.
I swapped the positioning here to better match the order of the other rules' tags.
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.
Look great! Just want @WilcoFiers to verify the categories are correct as he knows them best.
@@ -198,6 +198,10 @@ function createSchemas() { | |||
type: 'array', | |||
items: { | |||
type: 'string' | |||
}, | |||
conform: function hasCategoryTag(tags) { return tags.some(tag => tag.includes('cat.')) }, |
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.
Nice. I like this check. Thanks for adding this.
@mDalessandro so we merged a new rule and now this pr has a conflict. Sorry about that. Would you mind merging the latest develop to resolve the conflict? |
@straker no need. It's an easy fix. I've sorted it. |
Awesome, thanks for taking care of that. I haven't had a chance to sit down and do it, so that was helpful. I'll keep an eye out for any other issues I could work on. |
Add category tags to the rules that were missing them.
Add validation to check during a build for the inclusion of a category on the rule metadata.
Newly added categories for rules which were missing them:
aria-input-field-name:
cat.aria
aria-toggle-field-name:
cat.aria
aria-tooltip-name:
cat.aria
avoid-inline-spacing:
cat.structure
identical-links-same-purpose:
cat.semantics
label-content-name-mismatch:
cat.semantics
no-autoplay-audio:
cat.time-and-media
scrollable-region-focusable:
cat.keyboard
Let me know if you have a different thought on which categories should be used for these rules. Because there's no description of the categories themselves, it wasn't clear which one should be used; for example, both
cat.semantics
andcat.structure
seemed to have some overlap. That could probably be a new gihub issue if anyone else feels similarly.Closes issue #2554
Reviewer checks
Required fields, to be filled out by PR reviewer(s)