Add validateValues option to require-lang-attribute rule
#2478
Conversation
|
It looks like this is a breaking change right now? If so, we would want to hide this behavior behind an option |
bmish
changed the title
require-lang-attribute rule to validate attribute valuesvalidateValues option to require-lang-attribute rule
| ], | ||
| }); | ||
|
|
||
| generateRuleTests({ |
There was a problem hiding this comment.
Hmm, I think we probably want to avoid having an additional generateRuleTests call. Instead, we should pass config to specific test cases.
docs/rule/require-lang-attribute.md
Outdated
| @@ -29,7 +29,7 @@ This rule **forbids** the following: | |||
| ``` | |||
|
|
|||
| ```hbs | |||
| <html lang=""></html> | |||
| <html lang=''></html> | |||
There was a problem hiding this comment.
Usually we use double quotes in HTML, so probably avoid changing this?
docs/rule/require-lang-attribute.md
Outdated
| ## Configuration | ||
|
|
||
| - boolean -- if `true`, default configuration is applied | ||
| (`validateValues: false`), see below for details |
There was a problem hiding this comment.
Let's avoid repeating the default value here.
| (`validateValues: false`), see below for details |
docs/rule/require-lang-attribute.md
Outdated
|
|
||
| - object -- containing the following property: | ||
| - boolean -- `validateValues` -- if `true`, the rule checks whether the value in the `lang` attribute is a known IETF's BCP 47 language tag | ||
| (default: `false`) |
There was a problem hiding this comment.
We could indicate here that we should enable this by default in the next major release as a reminder:
| (default: `false`) | |
| (default: `false`) (TODO: enable by default in next major release) |
| @@ -50,3 +52,76 @@ generateRuleTests({ | |||
| }, | |||
| ], | |||
| }); | |||
|
|
|||
| generateRuleTests({ | |||
There was a problem hiding this comment.
Note that we want to test validateValues being explicitly true, explicitly false, and the default behavior when it's not specified at all. And we want to make it clear when we're testing each of these and make sure so that it will be easy to update the test cases when we change the default value to true in the next major release.
| good: [ | ||
| '<html lang="en"></html>', | ||
| '<html lang="en-US"></html>', | ||
| '<html lang={{lang}}></html>', |
There was a problem hiding this comment.
We need to make sure we have a test for the default behavior of allowing invalid values when an option isn't specified.
| '<html lang={{lang}}></html>', | |
| '<html lang={{lang}}></html>', | |
| '<html lang="hurrah"></html>' // allows invalid value when `validateValues` option defaults to `false` |
This PR fixes #2477.
Background
Currently, the
require-lang-attributerule checks for the presence of thelangattribute but not whether its value is valid. This issue proposes updating the rule so that, if thelangattribute contains a text value, check that its value is a known IETF's BCP 47 language tag. When this attribute is properly assigned, both assistive technologies and conventional user agents will be able to render text more accurately.Reference: lang - HTML: HyperText Markup Language - MDN Web Docs
Allowed:
Forbidden:
Testing
Test Suites: 136 passed, 136 total
Tests: 5 skipped, 7478 passed, 7483 total
Snapshots: 981 passed, 981 total
Time: 95.504 s
Ran all test suites.
✨ Done in 101.16s.