Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a

## [Unreleased]

### Added

- The `controlComponents` option for the `label-has-for` rule, which allows you to configure the rule to allow additional control components.

## [0.3.1] - 2020-07-10

### Changed
Expand Down
7 changes: 5 additions & 2 deletions docs/label-has-for.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ This rule takes one optional object argument of type object:
"vuejs-accessibility/label-has-for": [
"error",
{
"components": ["Label"],
"components": ["VLabel"],
"controlComponents": ["VInput"],
"required": {
"every": ["nesting", "id"]
},
Expand All @@ -28,7 +29,9 @@ This rule takes one optional object argument of type object:
}
```

For the `components` option, these strings determine which JSX elements (**always including** `<label>`) should be checked for having the `for` prop. This is a good use case when you have a wrapper component that simply renders a `label` element.
For the `components` option, these strings determine which elements (**always including** `<label>`) should be checked for having the `for` prop. This is a good use case when you have a wrapper component that simply renders a `label` element.

For the `controlComponents` option, these strings determine which elements should be counted as form control elements. By default, this includes `input`, `meter`, `progress`, `select`, and `textarea`. This is a good use case when you have a wrapper component that simplify renders a `input` element.

The `required` option (defaults to `"required": { "every": ["nesting", "id"] }`) determines which checks are activated. You're allowed to pass in one of the following types:

Expand Down
8 changes: 6 additions & 2 deletions src/rules/__tests__/label-has-for.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ makeRuleTester("label-has-for", rule, {
{
code: "<label for='id'><slot /></label>",
options: [{ allowChildren: true }]
},
{
code: "<label for='id'><VInput /></label>",
options: [{ controlComponents: ["VInput"] }]
}
],
invalid: [
Expand All @@ -21,8 +25,8 @@ makeRuleTester("label-has-for", rule, {
"<label for='id'><slot /></label>",
"<label for='id'><div /></label>",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be nice if we added a negative test for the custom input components?

{
code: "<Label for='id' />",
options: [{ components: ["Label"] }],
code: "<VLabel for='id' />",
options: [{ components: ["VLabel"] }],
errors: [{ messageId: "default" }]
}
]
Expand Down
2 changes: 1 addition & 1 deletion src/rules/__tests__/makeRuleTester.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const makeValidExample = (example) => {
return { filename, code: makeTemplate(example) };
}

return Object.assign(example, { filename });
return Object.assign(example, { filename, code: makeTemplate(example.code) });
};

const makeInvalidExample = (example) => {
Expand Down
32 changes: 23 additions & 9 deletions src/rules/label-has-for.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const {

const controlTypes = ["input", "meter", "progress", "select", "textarea"];

const validateNesting = (node, allowChildren) =>
const validateNesting = (node, allowChildren, controlComponents) =>
Copy link
Collaborator

@vhoyer vhoyer Aug 17, 2020

Choose a reason for hiding this comment

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

(optional) What do you think about having the controlComponents and the allowChildren both on the same second parameter as an object?

const validateNesting = (node, options) => {
  const { allowChildren, controlComponents } = options;

This way it would be easier to do the recursion and it would be more convinient to pass the arguments on the call stack on the rest of the file, meaning we would only need to write both, the allowChildren, and the controlComponents on the first call in the stack, the isValidLabel(node, required, { allowChildren, controlComponents }), the rest would only reference this options by "options"

node.children.some((child) => {
if (child.rawName === "slot") {
return allowChildren;
Expand All @@ -17,35 +17,41 @@ const validateNesting = (node, allowChildren) =>
if (child.type === "VElement") {
return (
!isHiddenFromScreenReader(child) &&
(controlTypes.includes(getElementType(child)) ||
(controlTypes
.concat(controlComponents)
.includes(getElementType(child)) ||
validateNesting(child, allowChildren))
Copy link
Collaborator

@vhoyer vhoyer Aug 17, 2020

Choose a reason for hiding this comment

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

(important) don't we need to pass the controlComponents to this calling of the funciton also?

);
}

return false;
});

const validate = (node, rule, allowChildren) => {
const validate = (node, rule, allowChildren, controlComponents) => {
switch (rule) {
case "nesting":
return validateNesting(node, allowChildren);
return validateNesting(node, allowChildren, controlComponents);
case "id":
return getElementAttributeValue(node, "for");
default:
return false;
}
};

const isValidLabel = (node, required, allowChildren) => {
const isValidLabel = (node, required, allowChildren, controlComponents) => {
if (Array.isArray(required.some)) {
return required.some.some((rule) => validate(node, rule, allowChildren));
return required.some.some((rule) =>
validate(node, rule, allowChildren, controlComponents)
);
}

if (Array.isArray(required.every)) {
return required.every.every((rule) => validate(node, rule, allowChildren));
return required.every.every((rule) =>
validate(node, rule, allowChildren, controlComponents)
);
}

return validate(node, required, allowChildren);
return validate(node, required, allowChildren, controlComponents);
};

module.exports = {
Expand All @@ -67,6 +73,13 @@ module.exports = {
},
uniqueItems: true
},
controlComponents: {
type: "array",
items: {
type: "string"
},
uniqueItems: true
},
required: {
oneOf: [
{
Expand Down Expand Up @@ -116,12 +129,13 @@ module.exports = {
const {
allowChildren = false,
components = [],
controlComponents = [],
required = { every: ["nesting", "id"] }
} = context.options[0] || {};

if (
["label"].concat(components).includes(getElementType(node)) &&
!isValidLabel(node, required, allowChildren)
!isValidLabel(node, required, allowChildren, controlComponents)
) {
context.report({ node, messageId: "default" });
}
Expand Down