-
Notifications
You must be signed in to change notification settings - Fork 788
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
Get develop-2x up to date with support docs #635
Conversation
f1b491c
to
419f909
Compare
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.
Looks good, although I wonder how you're going to pull the lookupTable changes back into 3.0.
// aria-valid-attr check | ||
"messages": { | ||
"pass": "ARIA attributes are used correctly for the defined role", | ||
"fail": "ARIA attribute{{=it.data && it.data.length > 1 ? 's are' : ' is'}} not allowed:{{~it.data:value}} {{=value}}{{~}}" |
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 have an incomplete
example?
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.
Probably, although I just took what was on jsdoc/develop.
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.
@WilcoFiers the previous sentence mentions the rule-development doc, which has thorough instructions for incomplete messages. I'd prefer not to have it in multiple places.
@@ -1,43 +1,55 @@ | |||
/*global aria, axe, lookupTables */ | |||
/*global aria, axe, lookupTable */ |
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.
We should pull these from aria.lookupTable
instead of relying on an implicit global.
@@ -1,61 +1,76 @@ | |||
/*global aria, lookupTables, axe */ | |||
/* global aria, lookupTable, axe */ |
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.
Same here, let's take this from aria.lookupTable
instead of using a global.
|
||
Occasionally, you may want to add additional information about why a Check passed or failed into its message. For example, the [aria-valid-attr](../lib/checks/aria/valid-attr.json) will add information about any invalid ARIA attributes to its fail message. The message uses the [doT.js](http://olado.github.io/doT/) and is compiled to a JavaScript function at build-time. In the Check message, you have access to the `CheckResult` as `it`. | ||
See [Developing Axe-core Rules](./rule-development.md) for more information |
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.
This section made it in twice somehow. Will remove.
@marcysutton Can you please add #559 also to the 2x branch. |
@iamrafan the changes are in there, but the history got lost. I'm trying to see if I can recreate the commit history without having to do the whole merge by hand again. |
I'm going to close this in favor of #636, which includes the commit history. |
In adding docs for #616 and #617, I wanted to start with Tony's great work on jsdoc. It was pretty intertwined with Shadow DOM, so I spent quite a bit of time merging. Fortunately, this process pulled in a few PRs that we'd missed on the 2x branch, most notably some ARIA 1.1 and color contrast message stuff. I was careful not to include anything related to Shadow DOM, but this would benefit from a thorough review just to make sure.
I think commons functions are best documented in the developer guide, so I added them there. We can a) add more details and b) include more commons functions, if desired. I think the jsdoc stuff Tony added helps a lot to clarify the source code.
Closes #616
Closes #617