-
Notifications
You must be signed in to change notification settings - Fork 791
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
feat: add EN.301.549 tags to rules #4063
Changes from all commits
c5e6523
d6febf9
2b6f0b6
96ee1c8
aa6be02
58c702d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,12 +198,6 @@ function createSchemas() { | |
type: 'array', | ||
items: { | ||
type: 'string' | ||
}, | ||
conform: function hasCategoryTag(tags) { | ||
return tags.some(tag => tag.includes('cat.')); | ||
}, | ||
messages: { | ||
conform: 'must include a category tag' | ||
} | ||
}, | ||
actIds: { | ||
|
@@ -307,5 +301,168 @@ function validateRule({ tags, metadata }) { | |
if (help.toLowerCase().includes(prohibitedWord)) { | ||
issues.push(`metadata.help can not contain the word '${prohibitedWord}'.`); | ||
} | ||
|
||
issues.push(...findTagIssues(tags)); | ||
return issues; | ||
} | ||
|
||
const miscTags = ['ACT', 'experimental', 'review-item', 'deprecated']; | ||
|
||
const categories = [ | ||
'aria', | ||
'color', | ||
'forms', | ||
'keyboard', | ||
'language', | ||
'name-role-value', | ||
'parsing', | ||
'semantics', | ||
'sensory-and-visual-cues', | ||
'structure', | ||
'tables', | ||
'text-alternatives', | ||
'time-and-media' | ||
]; | ||
|
||
const standardsTags = [ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's non-obvious that |
||
// Has to be first, as others rely on the WCAG level getting picked up first | ||
name: 'WCAG', | ||
straker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
standardRegex: /^wcag2(1|2)?a{1,3}(-obsolete)?$/, | ||
criterionRegex: /^wcag\d{3,4}$/ | ||
}, | ||
{ | ||
name: 'Section 508', | ||
standardRegex: /^section508$/, | ||
criterionRegex: /^section508\.\d{1,2}\.[a-z]$/, | ||
wcagLevelRegex: /^wcag2aa?$/ | ||
}, | ||
{ | ||
name: 'Trusted Tester', | ||
standardRegex: /^TTv5$/, | ||
criterionRegex: /^TT\d{1,3}\.[a-z]$/, | ||
wcagLevelRegex: /^wcag2aa?$/ | ||
}, | ||
{ | ||
name: 'EN 301 549', | ||
standardRegex: /^EN-301-549$/, | ||
criterionRegex: /^EN-9\.[1-4]\.[1-9]\.\d{1,2}$/, | ||
wcagLevelRegex: /^wcag21?aa?$/ | ||
} | ||
]; | ||
|
||
function findTagIssues(tags) { | ||
const issues = []; | ||
const catTags = tags.filter(tag => tag.startsWith('cat.')); | ||
const bestPracticeTags = tags.filter(tag => tag === 'best-practice'); | ||
|
||
// Category | ||
if (catTags.length !== 1) { | ||
issues.push(`Must have exactly one cat. tag, got ${catTags.length}`); | ||
} | ||
if (catTags.length && !categories.includes(catTags[0].slice(4))) { | ||
issues.push(`Invalid category tag: ${catTags[0]}`); | ||
} | ||
if (!startsWith(tags, catTags)) { | ||
issues.push(`Tag ${catTags[0]} must be before ${tags[0]}`); | ||
} | ||
tags = removeTags(tags, catTags); | ||
|
||
// Best practice | ||
if (bestPracticeTags.length > 1) { | ||
issues.push( | ||
`Only one best-practice tag is allowed, got ${bestPracticeTags.length}` | ||
); | ||
} | ||
if (!startsWith(tags, bestPracticeTags)) { | ||
issues.push(`Tag ${bestPracticeTags[0]} must be before ${tags[0]}`); | ||
} | ||
tags = removeTags(tags, bestPracticeTags); | ||
|
||
const standards = {}; | ||
// WCAG, Section 508, Trusted Tester, EN 301 549 | ||
for (const { | ||
name, | ||
standardRegex, | ||
criterionRegex, | ||
wcagLevelRegex | ||
} of standardsTags) { | ||
const standardTags = tags.filter(tag => tag.match(standardRegex)); | ||
const criterionTags = tags.filter(tag => tag.match(criterionRegex)); | ||
if (!standardTags.length && !criterionTags.length) { | ||
continue; | ||
} | ||
|
||
standards[name] = { | ||
name, | ||
standardTag: standardTags[0] ?? null, | ||
criterionTags | ||
}; | ||
if (bestPracticeTags.length !== 0) { | ||
issues.push(`${name} tags cannot be used along side best-practice tag`); | ||
} | ||
if (standardTags.length === 0) { | ||
issues.push(`Expected one ${name} tag, got 0`); | ||
} else if (standardTags.length > 1) { | ||
issues.push(`Expected one ${name} tag, got: ${standardTags.join(', ')}`); | ||
} | ||
if (criterionTags.length === 0) { | ||
issues.push(`Expected at least one ${name} criterion tag, got 0`); | ||
} | ||
|
||
if (wcagLevelRegex) { | ||
const wcagLevel = standards.WCAG.standardTag; | ||
WilcoFiers marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!wcagLevel.match(wcagLevelRegex)) { | ||
issues.push(`${name} rules not allowed on ${wcagLevel}`); | ||
} | ||
} | ||
|
||
// Must have the same criteria listed | ||
if (name === 'EN 301 549') { | ||
const wcagCriteria = standards.WCAG.criterionTags.map(tag => | ||
tag.slice(4) | ||
); | ||
const enCriteria = criterionTags.map(tag => | ||
tag.slice(5).replaceAll('.', '') | ||
); | ||
if ( | ||
wcagCriteria.length !== enCriteria.length || | ||
!startsWith(wcagCriteria, enCriteria) | ||
straker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) { | ||
issues.push( | ||
`Expect WCAG and EN criteria numbers to match: ${wcagCriteria.join( | ||
', ' | ||
)} vs ${enCriteria.join(', ')}}` | ||
); | ||
} | ||
} | ||
tags = removeTags(tags, [...standardTags, ...criterionTags]); | ||
} | ||
|
||
// Other tags | ||
const usedMiscTags = miscTags.filter(tag => tags.includes(tag)); | ||
const unknownTags = removeTags(tags, usedMiscTags); | ||
if (unknownTags.length) { | ||
issues.push(`Invalid tags: ${unknownTags.join(', ')}`); | ||
} | ||
|
||
// At this point only misc tags are left: | ||
tags = removeTags(tags, unknownTags); | ||
if (!startsWith(tags, usedMiscTags)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checks misc tags are in the correct order. The issue message can be better here though. I'll change that. |
||
issues.push( | ||
`Tags [${tags.join(', ')}] should be sorted like [${usedMiscTags.join( | ||
', ' | ||
)}]` | ||
); | ||
} | ||
|
||
return issues; | ||
} | ||
|
||
function startsWith(arr1, arr2) { | ||
return arr2.every((item, i) => item === arr1[i]); | ||
} | ||
|
||
function removeTags(tags, tagsToRemove) { | ||
return tags.filter(tag => !tagsToRemove.includes(tag)); | ||
} |
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.