feat: add EN.301.549 tags to rules#4063
Conversation
b543125 to
d6febf9
Compare
build/tasks/validate.js
Outdated
| name: 'Section 508', | ||
| standardRegex: /^section508$/, | ||
| criterionRegex: /^section508\.\d{1,2}\.[a-z]$/, | ||
| wcagLevelRegex: /wcag2aa?/ |
There was a problem hiding this comment.
Should the wcagLevelRegexes be terminated by ^ and $ to avoid, say, matching wcag2aaa by accident?
There was a problem hiding this comment.
Yes it should :) Fixed
build/tasks/validate.js
Outdated
| }, | ||
| { | ||
| name: 'EN 301 549', | ||
| standardRegex: /^EN.301.549$/, |
There was a problem hiding this comment.
| standardRegex: /^EN.301.549$/, | |
| standardRegex: /^EN\.301\.549$/, |
| ]; | ||
|
|
||
| const standardsTags = [ | ||
| { |
There was a problem hiding this comment.
It's non-obvious that findTagIssues relies on the WCAG entry being first in the list, maybe leave a comment noting that that's important?
straker
left a comment
There was a problem hiding this comment.
I think the tag validation should be its own PR and that we had some tests around the validation (might be hard since it's in a build step, but it's getting complex now that it's not strictly using the schema validator).
|
|
||
| // Other tags | ||
| const usedMiscTags = miscTags.filter(tag => tags.includes(tag)); | ||
| if (!startsWith(tags, usedMiscTags)) { |
There was a problem hiding this comment.
The only thing left in the array at this point is misc tags or unknown tags, so stating order seems irrelevant as the unknown tags will be flagged for removal.
There was a problem hiding this comment.
This checks misc tags are in the correct order. The issue message can be better here though. I'll change that.
| 'time-and-media' | ||
| ]; | ||
|
|
||
| const standardsTags = [ |
There was a problem hiding this comment.
Should we enforce that the order of tags follows this array? Right now it doesn't matter if section508 comes before EN-9, but does that matter?
There was a problem hiding this comment.
It does a little, yes. The order in which we list tags in the rule is the order in which they show up in other places. Having a consistent order will make it a little easier for people to find the tag they are looking for. It's minor, but it helps I think.
doc/API.md
Outdated
| | `section508.*.*` | Requirement in old Section 508 | | ||
| | `TTv5` | Trusted Tester v5 rules | | ||
| | `TT*.*` | Test ID in Trusted Tester | | ||
| | `EN.301.549` | Rule required under [EN 301 549](https://www.etsi.org/deliver/etsi_en/301500_301599/301549/03.02.01_60/en_301549v030201p.pdf) | |
There was a problem hiding this comment.
The . separators here feel a bit inconsistent with other tags as a separator for what are spaces in the official standard being referenced. Other existing cases either omit a separator (Section 508 -> section508) or use a hyphen (best-bractice). I think EN-301-549 is the nicest-looking consistent option, but I think any of EN301549, en301549, en-301-549 would be fine.
This feedback is minor; I don't think creates big backcompat issues, so if anyone else felt strongly about the .s, I can live with the inconsistency.
There was a problem hiding this comment.
Dashes make sense. @straker what do you think of EN-301-549?
doc/API.md
Outdated
| | `TTv5` | Trusted Tester v5 rules | | ||
| | `TT*.*` | Test ID in Trusted Tester | | ||
| | `EN.301.549` | Rule required under [EN 301 549](https://www.etsi.org/deliver/etsi_en/301500_301599/301549/03.02.01_60/en_301549v030201p.pdf) | | ||
| | `EN-9.*` | Section in EN 301 549 listing the requirement | |
There was a problem hiding this comment.
Using just EN without any other prefix makes me nervous about creating future backcompat issues; I think future revisions of EN 301 549 are likely to maintain consistent section numbering, but if there's ever a future different EN standard to, say, cover WCAG 3, it's reasonably likely it'd use conflicting numbering. That leads me to prefer using a fuller prefix here, like EN-301-549-9.1.2.3 or EN-301-549.9.1.2.3.
There was a problem hiding this comment.
We have that problem with other tags too. wcag111 won't be relevant when WCAG 3 comes out. TT1.a will probably be obsolete when Trusted Tester v6 comes out. Don't quite know what we're going to do at that point but it seems likely we need to come up with something different by that time.
I can well imagine we deprecate the criteria/section specific tags all together and put that information somewhere else. That's what we did with the ACT Rules, we added an actId prop to the rule.
There was a problem hiding this comment.
As discussed, the plan is to eventually not include SC numbers / section numbers into tags but make them available in some other format in the output. For now we're going with what we've been doing up until this point.
dbjorge
left a comment
There was a problem hiding this comment.
Thanks Wilco! I love the new validation, gave me much more confidence that my eyes didn't glaze over something during review. A few suggestions inline.
Closes: #4051