feat: moves html lang options from check to commons #357
Conversation
|
@dylanb @iandotkelly @marcysutton @WilcoFiers I'm working on tests, atm, but heads-up! |
|
@dylanb @iandotkelly @marcysutton @WilcoFiers Ready for your review. |
lib/commons/utils/valid-langs.js
Outdated
| /*global axe */ | ||
|
|
||
| axe.utils.validLangs = function () { | ||
| 'use strict'; |
There was a problem hiding this comment.
I don't think we need use strict as Babel adds it for us...as an FYI
There was a problem hiding this comment.
Some of the other utils have it, but happy to remove if you advise.
| @@ -43,13 +43,12 @@ describe('valid-lang', function () { | |||
| assert.deepEqual(checkContext._data, ['lang="en-FOO"']); | |||
| }); | |||
|
|
|||
| it('should return true (and not throw) when given no options', function () { | |||
| it('should return false (and not throw) when given no options', function () { | |||
There was a problem hiding this comment.
How come the return values changed here?
There was a problem hiding this comment.
It's related to the none in the check. It's a bit backwards. Previously, there were options or [], but now we've got (options ? options : axe.commons.utils.validLangs) - so either null or some, but never empty.
This is my best explanation. It's kind of hard to wrap my head around the tests that are true, that return false :)
| @@ -43,13 +43,12 @@ describe('valid-lang', function () { | |||
| assert.deepEqual(checkContext._data, ['lang="en-FOO"']); | |||
| }); | |||
|
|
|||
| it('should return true (and not throw) when given no options', function () { | |||
| it('should return false (and not throw) when given no options', function () { | |||
There was a problem hiding this comment.
Add a test that tests to make sure it returns true if passed an empty options array
There was a problem hiding this comment.
@dylanb doesn't the test on line 37 do that?
|
@downzer0 - as mentioned in the call - the list of languages is built by the |
|
@dylanb Everything seems to build OK, and when I manually bring in the compiled Should we remove the Grunt task? |
|
No, the grunt task does not get called automatically, it is there to build the list should the languages change - now admittedly, this happens very infrequently, but calling it periodically will automatically update for changes whereas maintaining that manually is almost impossible due to the size. So the Grunt task should be changed to generate the code in the commons from a template (or something equivalent) |
|
@dylanb I've made an update which alters the Gruntfile location to the new util file. I originally had this generate a JSON file and then tried I've tested the build process and tested the output axe.js in our rulebuilder. |
|
So this task for running langs...we should document it better. There is a mention of But I'm also wondering if we should register a task at the bottom of the Gruntfile to make this more obvious. |
|
@marcysutton Good catch, with the documentation - I'll get it updated. My only hesitation with the registered task is that, based on @dylanb's comment, it doesn't sound like this is used much. Also, you can run it with |
|
I didn't even know it was there, since there is another variable called |
|
Just looked - the docs don't need an update. I will get the Grunt task documented though. |
|
We will document the Grunt tasks in a separate PR. Marcy is creating an issue for it. |
|
Looks good! Circle just needed a rebuild. |
* feat(wdjs): Add .setLegacyMode * feat(wdio): add setLegacyMode * feat(puppeteer): Add setLegacyMode * feat(playwright): Add setLegacyMode * chore: upgrade package-lock * docs(puppeteer): add doc blocks to all methods
The aXe rule/check for valid HTML
langattributes had an options array that was being included three times not only making theaxe.jsfile three times larger than it needed to be, but also made other apps that consume aXe at least that much larger as well.I've moved the options array from the check to an
axe.commons.utilsfile that returns the same and updated the check JavaScript itself to useoptionsif present, otherwise fallback to the returned array of every possible language code.Closes #348.