Skip to content
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: deprecate the use doT.js for messages #1938

Merged
merged 11 commits into from
Dec 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Axe can be built using your local language. To do so, a localization file must b

This will create a new build for axe, called `axe.<lang>.js` and `axe.<lang>.min.js`. If you want to build localized versions, simply pass in `--all-lang` instead.

To create a new translation for axe, start by running `grunt translate --lang=<langcode>`. This will create a json file fin the `./locales` directory, with the default English text in it for you to translate. We welcome any localization for axe-core. For details on how to contribute, see the Contributing section below.
To create a new translation for axe, start by running `grunt translate --lang=<langcode>`. This will create a json file fin the `./locales` directory, with the default English text in it for you to translate. We welcome any localization for axe-core. For details on how to contribute, see the Contributing section below. For details on the message syntax, see [Check Message Template](/docs/check-message-template.md).

To update existing translation file, re-run `grunt translate --lang=<langcode>`. This will add new messages used in English and remove messages which were not used in English.

Expand All @@ -116,7 +116,7 @@ axe.configure({
'aria-errormessage': {
// Note: doT (https://github.com/olado/dot) templates are supported here.
fail:
'Der Wert der aria-errormessage {{~it.data:value}} `{{=value}}{{~}}` muss eine Technik verwenden, um die Message anzukündigen (z. B., aria-live, aria-describedby, role=alert, etc.).'
'Der Wert der aria-errormessage ${data.values}` muss eine Technik verwenden, um die Message anzukündigen (z. B., aria-live, aria-describedby, role=alert, etc.).'
}
// ...
}
Expand Down
13 changes: 10 additions & 3 deletions build/configure.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var dot = require('@deque/dot');
var templates = require('./templates');
var buildManual = require('./build-manual');
var entities = new (require('html-entities')).AllHtmlEntities();
var dotRegex = /\{\{.+?\}\}/g;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think I'm familiar with a .+? in regex, what does that do?

Copy link
Contributor Author

@straker straker Dec 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ? makes the .+ not greedy. So instead of {{foo bar}} {{foo}} matching the entire string, it will stop at the first set of }} so you get 2 matches. Not that it really matters in this case but I'm use to making things not greedy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, didn't know that. Cool


var descriptionHeaders =
'| Rule ID | Description | Impact | Tags | Enabled by default | Failures | Needs Review |\n| :------- | :------- | :------- | :------- | :------- | :------- | :------- |\n';
Expand Down Expand Up @@ -59,15 +60,18 @@ function buildRules(grunt, options, commons, callback) {
Object.keys(result.messages).forEach(function(key) {
// only convert to templated function for strings
// objects handled later in publish-metadata.js
if (typeof result.messages[key] !== 'object') {
if (
typeof result.messages[key] !== 'object' &&
dotRegex.test(result.messages[key])
) {
result.messages[key] = dot
.template(result.messages[key])
.toString();
}
});
}
//TODO this is actually failureSummaries, property name should better reflect that
if (result.failureMessage) {
if (result.failureMessage && dotRegex.test(result.failureMessage)) {
result.failureMessage = dot.template(result.failureMessage).toString();
}
return result;
Expand All @@ -86,7 +90,10 @@ function buildRules(grunt, options, commons, callback) {
function getIncompleteMsg(summaries) {
var result = {};
summaries.forEach(function(summary) {
if (summary.incompleteFallbackMessage) {
if (
summary.incompleteFallbackMessage &&
dotRegex.test(summary.incompleteFallbackMessage)
) {
result = dot.template(summary.incompleteFallbackMessage).toString();
}
});
Expand Down
2 changes: 0 additions & 2 deletions build/tasks/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ function hasMultipleOutcomes(messages) {
switch (key) {
case 'pass':
case 'fail':
return typeof messages[key] === 'string';

case 'incomplete':
return ['string', 'object'].includes(typeof messages[key]);

Expand Down
124 changes: 124 additions & 0 deletions doc/check-message-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
# Check Message Template

Axe-core uses a custom template to handle dynamic check messages (messages that use the `data` property to output values or to determine which message to display). The structure for the messages is as follows:

## Simple Message

A simple message is just a string that doesn't use the `data` property. Most checks uses this format.

```json
{
"messages": {
"pass": "Simple message for a passing check"
}
}
```

## Message with Data

A message can also use the `data` property to output information from the check. If `data` is a String, Boolean, or Number, you can use the syntax `${data}` to have the message output the value of the `data` property.

```js
// check.js
this.data(10);

// check.json
{
"messages": {
"pass": "Passed with a value of ${data}"
// => "Passed with a value of 10"
}
}
```

If `data` is an object, you can access properties of the object using the syntax `${data.propName}`.

```js
// check.js
this.data({
contrast: '3:1',
fontSize: '12px'
});

// check.json
{
"messages": {
"fail": "Color-contrast failed with a contrast of ${data.contrast} and font size of ${data.fontSize}"
// => "Color-contrast failed with a contrast of 3:1 and font size of 12px"
}
}
```

## Singular and Plural Messages

If the message needs to to know how many items are in the `data` property to determine the type of language to use (singular or plural), you can structure the message to use `singular` and `plural` properties. Use the syntax `${data.values}` to have the message output a comma-separated list of the items (`data.values` is provided by the template code for you).

```js
// check.js
this.data(['item1', 'item2']);

// check.json
{
"messages": {
"fail": {
"singular": "Attribute ${data.values} is not allowed",
"plural": "Attributes: ${data.values} are not allowed"
}
// => Attributes: item1, item2 are not allowed
}
}
```

## Message Determined by Data

Lastly, a message can use the `data` property to determine which message to display. Structure the message to use properties whose keys are the possible values of `data.messageKey`. You should also provide a `default` message that will be displayed if `messageKey` is not set.

```js
// check.js
this.data({
messageKey: 'imgNode'
});

// check.json
{
"messages": {
"incomplete": {
"default": "Color-contrast could not be determined"
"bgImage": "Element's background color could not be determined due to a background image",
"imgNode": "Element's background color could not be determined because element contains an image node"
}
// => Element's background color could not be determined because element contains an image node
}
}
```

The messages can still use the syntax `${data.propName}` to access other properties on the `data` property.

## Migrating From doT.js Template in Translations

Axe-core use to use doT.js for it's temple library. To migrate from doT.js in a translation file, do the following:

- If the message used `{{=it.data}}` or `{{=it.data.propName}}`, change the message to use the syntax `${data}` or `${data.propName}`.

```diff
{
"messages": {
- "incomplete": "Check that the <label> does not need be part of the ARIA {{=it.data}} field's name"
+ "incomplete": "Check that the <label> does not need be part of the ARIA ${data} field's name"
}
}
```

- If the message used `{{=it.data && it.data.length` to determine using singular or plural language, change the message structure of the message to instead use the `singular` and `plural` properties. Replace `{{=it.data.join(', ')}}` with `${data.values}`.

```diff
{
"messages": {
- "fail": "Attribute{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} {{=it.data && it.data.length > 1 ? 'are' : ' is'}} not allowed
+ "fail": {
+ "singular": "Attribute ${data.values} is not allowed",
+ "plural": "Attributes: ${data.values} are not allowed"
+ }
}
}
```
5 changes: 4 additions & 1 deletion lib/checks/aria/allowed-attr.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"impact": "critical",
"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}}{{~}}"
"fail": {
"singular": "ARIA attribute is not allowed: ${data.values}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is 1,000,000x easier to read/follow ❤️

"plural": "ARIA attributes are not allowed: ${data.values}"
}
}
}
}
10 changes: 8 additions & 2 deletions lib/checks/aria/aria-allowed-role.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@
"impact": "minor",
"messages": {
"pass": "ARIA role is allowed for given element",
"fail": "ARIA role{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} {{=it.data && it.data.length > 1 ? 'are' : ' is'}} not allowed for given element",
"incomplete": "ARIA role{{=it.data && it.data.length > 1 ? 's' : ''}} {{=it.data.join(', ')}} must be removed when the element is made visible, as {{=it.data && it.data.length > 1 ? 'they are' : 'it is'}} not allowed for the element"
"fail": {
"singular": "ARIA role ${data.values} is not allowed for given element",
"plural": "ARIA roles ${data.values} are not allowed for given element"
},
"incomplete": {
"singular": "ARIA role ${data.values} must be removed when the element is made visible, as it is not allowed for the element",
"plural": "ARIA roles ${data.values} must be removed when the element is made visible, as they are not allowed for the element"
}
}
}
}
5 changes: 4 additions & 1 deletion lib/checks/aria/errormessage.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"impact": "critical",
"messages": {
"pass": "Uses a supported aria-errormessage technique",
"fail": "aria-errormessage value{{=it.data && it.data.length > 1 ? 's' : ''}} {{~it.data:value}} `{{=value}}{{~}}` must use a technique to announce the message (e.g., aria-live, aria-describedby, role=alert, etc.)"
"fail": {
"singular": "aria-errormessage value `${data.values}` must use a technique to announce the message (e.g., aria-live, aria-describedby, role=alert, etc.)",
"plural": "aria-errormessage values `${data.values}` must use a technique to announce the message (e.g., aria-live, aria-describedby, role=alert, etc.)"
}
}
}
}
2 changes: 1 addition & 1 deletion lib/checks/aria/no-implicit-explicit-label.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"impact": "moderate",
"messages": {
"pass": "There is no mismatch between a <label> and accessible name",
"incomplete": "Check that the <label> does not need be part of the ARIA {{=it.data}} field's name"
"incomplete": "Check that the <label> does not need be part of the ARIA ${data} field's name"
}
}
}
5 changes: 4 additions & 1 deletion lib/checks/aria/required-attr.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"impact": "critical",
"messages": {
"pass": "All required ARIA attributes are present",
"fail": "Required ARIA attribute{{=it.data && it.data.length > 1 ? 's' : ''}} not present:{{~it.data:value}} {{=value}}{{~}}"
"fail": {
"singular": "Required ARIA attribute not present: ${data.values}",
"plural": "Required ARIA attributes not present: ${data.values}"
}
}
}
}
10 changes: 8 additions & 2 deletions lib/checks/aria/required-children.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@
"impact": "critical",
"messages": {
"pass": "Required ARIA children are present",
"fail": "Required ARIA {{=it.data && it.data.length > 1 ? 'children' : 'child'}} role not present:{{~it.data:value}} {{=value}}{{~}}",
"incomplete": "Expecting ARIA {{=it.data && it.data.length > 1 ? 'children' : 'child'}} role to be added:{{~it.data:value}} {{=value}}{{~}}"
"fail": {
"singular": "Required ARIA child role not present: ${data.values}",
"plural": "Required ARIA children role not present: ${data.values}"
},
"incomplete": {
"singular": "Expecting ARIA child role to be added: ${data.values}",
"plural": "Expecting ARIA children role to be added: ${data.values}"
}
}
}
}
5 changes: 4 additions & 1 deletion lib/checks/aria/required-parent.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"impact": "critical",
"messages": {
"pass": "Required ARIA parent role present",
"fail": "Required ARIA parent{{=it.data && it.data.length > 1 ? 's' : ''}} role not present:{{~it.data:value}} {{=value}}{{~}}"
"fail": {
"singular": "Required ARIA parent role not present: ${data.values}",
"plural": "Required ARIA parents role not present: ${data.values}"
}
}
}
}
2 changes: 1 addition & 1 deletion lib/checks/aria/unsupportedattr.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"impact": "critical",
"messages": {
"pass": "ARIA attribute is supported",
"fail": "ARIA attribute is not widely supported in screen readers and assistive technologies: {{~it.data:value}} {{=value}}{{~}}"
"fail": "ARIA attribute is not widely supported in screen readers and assistive technologies: ${data.values}"
}
}
}
2 changes: 1 addition & 1 deletion lib/checks/aria/unsupportedrole.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"impact": "critical",
"messages": {
"pass": "ARIA role is supported",
"fail": "The role used is not widely supported in screen readers and assistive technologies: {{~it.data:value}} {{=value}}{{~}}"
"fail": "The role used is not widely supported in screen readers and assistive technologies: ${data.values}"
}
}
}
10 changes: 8 additions & 2 deletions lib/checks/aria/valid-attr-value.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@
"impact": "critical",
"messages": {
"pass": "ARIA attribute values are valid",
"fail": "Invalid ARIA attribute value{{=it.data && it.data.length > 1 ? 's' : ''}}:{{~it.data:value}} {{=value}}{{~}}",
"incomplete": "ARIA attribute{{=it.data && it.data.length > 1 ? 's' : ''}} element ID does not exist on the page:{{~it.data:value}} {{=value}}{{~}}"
"fail": {
"singular": "Invalid ARIA attribute value: ${data.values}",
"plural": "Invalid ARIA attribute values: ${data.values}"
},
"incomplete": {
"singular": "ARIA attribute element ID does not exist on the page: ${data.values}",
"plural": "ARIA attributes element ID does not exist on the page: ${data.values}"
}
}
}
}
7 changes: 5 additions & 2 deletions lib/checks/aria/valid-attr.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
"metadata": {
"impact": "critical",
"messages": {
"pass": "ARIA attribute name{{=it.data && it.data.length > 1 ? 's' : ''}} are valid",
"fail": "Invalid ARIA attribute name{{=it.data && it.data.length > 1 ? 's' : ''}}:{{~it.data:value}} {{=value}}{{~}}"
"pass": "ARIA attribute name is valid",
"fail": {
"singular": "Invalid ARIA attribute name: ${data.values}",
"plural": "Invalid ARIA attribute names: ${data.values}"
}
}
}
}
2 changes: 1 addition & 1 deletion lib/checks/color/color-contrast.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const data = {
contrastRatio: cr ? truncatedResult : undefined,
fontSize: `${((fontSize * 72) / 96).toFixed(1)}pt (${fontSize}px)`,
fontWeight: bold ? 'bold' : 'normal',
missingData: missing,
messageKey: missing,
expectedContrastRatio: cr.expectedContrastRatio + ':1'
};

Expand Down
8 changes: 4 additions & 4 deletions lib/checks/color/color-contrast.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
"metadata": {
"impact": "serious",
"messages": {
"pass": "Element has sufficient color contrast of {{=it.data.contrastRatio}}",
"fail": "Element has insufficient color contrast of {{=it.data.contrastRatio}} (foreground color: {{=it.data.fgColor}}, background color: {{=it.data.bgColor}}, font size: {{=it.data.fontSize}}, font weight: {{=it.data.fontWeight}}). Expected contrast ratio of {{=it.data.expectedContrastRatio}}",
"pass": "Element has sufficient color contrast of ${data.contrastRatio}",
"fail": "Element has insufficient color contrast of ${data.contrastRatio} (foreground color: ${data.fgColor}, background color: ${data.bgColor}, font size: ${data.fontSize}, font weight: ${data.fontWeight}). Expected contrast ratio of ${data.expectedContrastRatio}",
"incomplete": {
"default": "Unable to determine contrast ratio",
"bgImage": "Element's background color could not be determined due to a background image",
"bgGradient": "Element's background color could not be determined due to a background gradient",
"imgNode": "Element's background color could not be determined because element contains an image node",
Expand All @@ -16,8 +17,7 @@
"elmPartiallyObscuring": "Element's background color could not be determined because it partially overlaps other elements",
"outsideViewport": "Element's background color could not be determined because it's outside the viewport",
"equalRatio": "Element has a 1:1 contrast ratio with the background",
"shortTextContent": "Element content is too short to determine if it is actual text content",
"default": "Unable to determine contrast ratio"
"shortTextContent": "Element content is too short to determine if it is actual text content"
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/checks/color/link-in-text-block.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ if (color.elementIsDistinct(node, parentBlock)) {
} else if (contrast >= 3.0) {
axe.commons.color.incompleteData.set('fgColor', 'bgContrast');
this.data({
missingData: axe.commons.color.incompleteData.get('fgColor')
messageKey: axe.commons.color.incompleteData.get('fgColor')
});
axe.commons.color.incompleteData.clear();
// User needs to check whether there is a hover and a focus style
Expand All @@ -74,7 +74,7 @@ if (color.elementIsDistinct(node, parentBlock)) {
}
axe.commons.color.incompleteData.set('fgColor', reason);
this.data({
missingData: axe.commons.color.incompleteData.get('fgColor')
messageKey: axe.commons.color.incompleteData.get('fgColor')
});
axe.commons.color.incompleteData.clear();
return undefined;
Expand Down
4 changes: 2 additions & 2 deletions lib/checks/color/link-in-text-block.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
"pass": "Links can be distinguished from surrounding text in some way other than by color",
"fail": "Links need to be distinguished from surrounding text in some way other than by color",
"incomplete": {
"default": "Unable to determine contrast ratio",
"bgContrast": "Element's contrast ratio could not be determined. Check for a distinct hover/focus style",
"bgImage": "Element's contrast ratio could not be determined due to a background image",
"bgGradient": "Element's contrast ratio could not be determined due to a background gradient",
"imgNode": "Element's contrast ratio could not be determined because element contains an image node",
"bgOverlap": "Element's contrast ratio could not be determined because of element overlap",
"default": "Unable to determine contrast ratio"
"bgOverlap": "Element's contrast ratio could not be determined because of element overlap"
}
}
}
Expand Down