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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ Any use of third-party trademarks or logos are subject to those third-party's po
| [emptyswatch-needs-labelling](docs/rules/emptyswatch-needs-labelling.md) | Accessibility: EmptySwatch must have an accessible name via aria-label, Tooltip, aria-labelledby, etc.. | ✅ | | |
| [field-needs-labelling](docs/rules/field-needs-labelling.md) | Accessibility: Field must have label | ✅ | | |
| [image-button-missing-aria](docs/rules/image-button-missing-aria.md) | Accessibility: Image buttons must have accessible labelling: title, aria-label, aria-labelledby, aria-describedby | ✅ | | |
| [image-needs-alt](docs/rules/image-needs-alt.md) | Accessibility: Image must have alt attribute with a meaningful description of the image. If the image is decorative, use alt="". | ✅ | | |
| [imageswatch-needs-labelling](docs/rules/imageswatch-needs-labelling.md) | Accessibility: ImageSwatch must have an accessible name via aria-label, Tooltip, aria-labelledby, etc.. | ✅ | | |
| [input-components-require-accessible-name](docs/rules/input-components-require-accessible-name.md) | Accessibility: Input fields must have accessible labelling: aria-label, aria-labelledby or an associated label | ✅ | | |
| [link-missing-labelling](docs/rules/link-missing-labelling.md) | Accessibility: Image links must have an accessible name. Add either text content, labelling to the image or labelling to the link itself. | ✅ | | 🔧 |
Expand Down
34 changes: 34 additions & 0 deletions docs/rules/image-needs-alt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Accessibility: Image must have alt attribute with a meaningful description of the image. If the image is decorative, use alt="" (`@microsoft/fluentui-jsx-a11y/image-needs-alt`)

💼 This rule is enabled in the ✅ `recommended` config.

<!-- end auto-generated rule header -->

## Rule details

This rule requires all `<Image>` components have non-empty alternative text. The `alt` attribute should provide a clear and concise text replacement for the image's content. It should *not* describe the presence of the image itself or the file name of the image. Purely decorative images should have empty `alt` text (`alt=""`).


Examples of **incorrect** code for this rule:

```jsx
<Image src="image.png" />
```

```jsx
<Image src="image.png" alt={null} />
```

Examples of **correct** code for this rule:

```jsx
<Image src="image.png" alt="A dog playing in a park." />
```

```jsx
<Image src="decorative-image.png" alt="" />
```

## Further Reading

- [`<img>` Accessibility](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/img#accessibility)
2 changes: 2 additions & 0 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ module.exports = {
"@microsoft/fluentui-jsx-a11y/emptyswatch-needs-labelling": "error",
"@microsoft/fluentui-jsx-a11y/field-needs-labelling": "error",
"@microsoft/fluentui-jsx-a11y/image-button-missing-aria": "error",
"@microsoft/fluentui-jsx-a11y/image-needs-alt": "error",
"@microsoft/fluentui-jsx-a11y/imageswatch-needs-labelling": "error",
"@microsoft/fluentui-jsx-a11y/input-components-require-accessible-name": "error",
"@microsoft/fluentui-jsx-a11y/link-missing-labelling": "error",
Expand Down Expand Up @@ -74,6 +75,7 @@ module.exports = {
"emptyswatch-needs-labelling": rules.emptySwatchNeedsLabelling,
"field-needs-labelling": rules.fieldNeedsLabelling,
"image-button-missing-aria": rules.imageButtonMissingAria,
"image-needs-alt": rules.imageNeedsAlt,
"imageswatch-needs-labelling": rules.imageSwatchNeedsLabelling,
"input-components-require-accessible-name": rules.inputComponentsRequireAccessibleName,
"link-missing-labelling": rules.linkMissingLabelling,
Expand Down
28 changes: 28 additions & 0 deletions lib/rules/image-needs-alt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { ESLintUtils } from "@typescript-eslint/utils";
import { makeLabeledControlRule } from "../util/ruleFactory";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const rule = ESLintUtils.RuleCreator.withoutDocs(
makeLabeledControlRule({
component: "Image",
messageId: "imageNeedsAlt",
description:
'Accessibility: Image must have alt attribute with a meaningful description of the image. If the image is decorative, use alt="".',
requiredProps: ["alt"],
allowFieldParent: false,
allowHtmlFor: false,
allowLabelledBy: false,
allowWrappingLabel: false,
allowTooltipParent: false,
allowDescribedBy: false,
allowLabeledChild: false
})
);

export default rule;
1 change: 1 addition & 0 deletions lib/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export { default as dialogsurfaceNeedsAria } from "./dialogsurface-needs-aria";
export { default as dropdownNeedsLabelling } from "./dropdown-needs-labelling";
export { default as fieldNeedsLabelling } from "./field-needs-labelling";
export { default as imageButtonMissingAria } from "./buttons/image-button-missing-aria";
export { default as imageNeedsAlt } from "./image-needs-alt";
export { default as inputComponentsRequireAccessibleName } from "./input-components-require-accessible-name";
export { default as linkMissingLabelling } from "./link-missing-labelling";
export { default as menuItemNeedsLabelling } from "./menu-item-needs-labelling";
Expand Down
43 changes: 43 additions & 0 deletions lib/util/hasDefinedProp.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { TSESTree } from "@typescript-eslint/utils";
import { JSXOpeningElement } from "estree-jsx";
import { hasProp, getPropValue, getProp } from "jsx-ast-utils";

/**
* Determines if the property exists and has a non-nullish value.
* @param attributes The attributes on the visited node
* @param name The name of the prop to check
* @returns Whether the specified prop exists and is not null or undefined
* @example
* // <img src="image.png" />
* hasDefinedProp(attributes, 'src') // true
* // <img src="" />
* hasDefinedProp(attributes, 'src') // true
* // <img src={null} />
* hasDefinedProp(attributes, 'src') // false
* // <img src={undefined} />
* hasDefinedProp(attributes, 'src') // false
* // <img src={1} />
* hasDefinedProp(attributes, 'src') // false
* // <img src={true} />
* hasDefinedProp(attributes, 'src') // false
* // <img />
* hasDefinedProp(attributes, 'src') // false
*/
const hasDefinedProp = (attributes: TSESTree.JSXOpeningElement["attributes"], name: string): boolean => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there is already a function that checks for a non-empty prop? Maybe that function could be adjusted to include null and undefined checks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's hasNonEmptyProp but adding a flag to that to specifically allow an empty value seemed counter-intuitive. Could rename it from hasNonEmptyProp to hasDefinedProp with a flag to allow non-empty values, but that would require a lot of files to be touched for the name change and I was trying to keep this PR small-ish. If you feel strongly about this I could implement this in a follow-up PR?

if (!hasProp(attributes as JSXOpeningElement["attributes"], name)) {
return false;
}

const prop = getProp(attributes as JSXOpeningElement["attributes"], name);

// Safely get the value of the prop, handling potential undefined or null values
const propValue = prop ? getPropValue(prop) : undefined;

// Return true if the prop value is not null or undefined
return propValue !== null && propValue !== undefined;
};

export { hasDefinedProp };
55 changes: 39 additions & 16 deletions lib/util/ruleFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,43 @@ import { elementType } from "jsx-ast-utils";
import { JSXOpeningElement } from "estree-jsx";
import { hasToolTipParent } from "./hasTooltipParent";
import { hasLabeledChild } from "./hasLabeledChild";
import { hasDefinedProp } from "./hasDefinedProp";
import { hasTextContentChild } from "./hasTextContentChild";

/**
* Configuration options for a rule created via the `ruleFactory`
*/
export type LabeledControlConfig = {
/** The name of the component that the rule applies to. @example 'Image', /Image|Icon/ */
component: string | RegExp;
/** The unique id of the problem message. @example 'itemNeedsLabel' */
messageId: string;
/** A short description of the rule. */
description: string;
labelProps: string[]; // e.g. ["aria-label", "title", "label"]
allowFieldParent: boolean; // Accept a parent <Field label="..."> wrapper as providing the label.
allowHtmlFor: boolean; // Accept <label htmlFor="..."> association.
allowLabelledBy: boolean; // Accept aria-labelledby association.
allowWrappingLabel: boolean; // Accept being wrapped in a <label> element.
allowTooltipParent: boolean; // Accept a parent <Tooltip content="..."> wrapper as providing the label.
/** Properties that are required to have a non-`null` and non-`undefined` value. @example ["alt"] */
requiredProps?: string[];
/** Labeling properties that are required to have at least one non-empty value. @example ["aria-label", "title", "label"] */
labelProps?: string[];
/** Accept a parent `<Field label="...">` wrapper as providing the label. */
allowFieldParent: boolean;
/** Accept `<label htmlFor="...">` association. */
allowHtmlFor: boolean;
/** Accept aria-labelledby association. */
allowLabelledBy: boolean;
/** Accept being wrapped in a `<label>` element. */
allowWrappingLabel: boolean;
/** Accept a parent `<Tooltip content="...">` wrapper as providing the label. */
allowTooltipParent: boolean;
/**
* Accept aria-describedby as a labeling strategy.
* NOTE: This is discouraged for *primary* labeling; prefer text/aria-label/labelledby.
* Keep this off unless a specific component (e.g., Icon-only buttons) intentionally uses it.
*/
allowDescribedBy: boolean;
allowLabeledChild: boolean; // Accept labeled child elements to provide the label e.g. <Button><img alt="..." /></Button>
allowTextContentChild?: boolean; // Accept text children to provide the label e.g. <Button>Click me</Button>
/** Treat labeled child content (img `alt`, svg `title`, `aria-label` on `role="img"`) as the name */
allowLabeledChild: boolean;
/** Accept text children to provide the label e.g. <Button>Click me</Button> */
allowTextContentChild?: boolean;
};

/**
Expand All @@ -58,16 +75,22 @@ export function hasAccessibleLabel(
context: TSESLint.RuleContext<string, []>,
config: LabeledControlConfig
): boolean {
const allowFieldParent = !!config.allowFieldParent;
const allowWrappingLabel = !!config.allowWrappingLabel;
const allowHtmlFor = !!config.allowHtmlFor;
const allowLabelledBy = !!config.allowLabelledBy;
const allowTooltipParent = !!config.allowTooltipParent;
const allowDescribedBy = !!config.allowDescribedBy;
const allowLabeledChild = !!config.allowLabeledChild;
const {
allowFieldParent,
allowWrappingLabel,
allowHtmlFor,
allowLabelledBy,
allowTooltipParent,
allowDescribedBy,
allowLabeledChild,
requiredProps,
labelProps
} = config;
const allowTextContentChild = !!config.allowTextContentChild;

if (allowFieldParent && hasFieldParent(context)) return true;
if (config.labelProps?.some(p => hasNonEmptyProp(opening.attributes, p))) return true;
if (requiredProps?.every(p => hasDefinedProp(opening.attributes, p))) return true;
if (labelProps?.some(p => hasNonEmptyProp(opening.attributes, p))) return true;
if (allowWrappingLabel && isInsideLabelTag(context)) return true;
if (allowHtmlFor && hasAssociatedLabelViaHtmlFor(opening, context)) return true;
if (allowLabelledBy && hasAssociatedLabelViaAriaLabelledBy(opening, context)) return true;
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
"lint:eslint-docs": "npm-run-all \"update:eslint-docs -- --check\"",
"lint:js": "eslint .",
"test": "jest",
"test:branch": "npm run test -- -o",
"test:watch": "npm run test -- --watch",
"lint:docs": "markdownlint **/*.md",
"update:eslint-docs": "eslint-doc-generator",
"fix:md": "npm run lint:docs -- --fix",
Expand Down
7 changes: 5 additions & 2 deletions scripts/boilerplate/doc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

const docBoilerplateGenerator = (name, description) => `# ${description} (@microsoft/fluentui-jsx-a11y/${name})
const { withCRLF } = require("./util");

const docBoilerplateGenerator = (name, description) =>
withCRLF(`# ${description} (@microsoft/fluentui-jsx-a11y/${name})

Write a useful explanation here!

Expand All @@ -18,5 +21,5 @@ Write more details here!
\`\`\`

## Further Reading
`;
`);
module.exports = docBoilerplateGenerator;
7 changes: 5 additions & 2 deletions scripts/boilerplate/rule.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

const ruleBoilerplate = (name, description) => `// Copyright (c) Microsoft Corporation.
const { withCRLF } = require("./util");

const ruleBoilerplate = (name, description) =>
withCRLF(`// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { ESLintUtils, TSESTree } from "@typescript-eslint/utils";
Expand Down Expand Up @@ -41,5 +44,5 @@ const rule = createRule({
});

export default rule;
`;
`);
module.exports = ruleBoilerplate;
7 changes: 5 additions & 2 deletions scripts/boilerplate/test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

const testBoilerplate = name => `// Copyright (c) Microsoft Corporation.
const { withCRLF } = require("./util");

const testBoilerplate = name =>
withCRLF(`// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { Rule } from "eslint";
Expand All @@ -20,5 +23,5 @@ ruleTester.run("${name}", rule as unknown as Rule.RuleModule, {
/* ... */
]
});
`;
`);
module.exports = testBoilerplate;
9 changes: 9 additions & 0 deletions scripts/boilerplate/util.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Helper method to convert LF line endings to CRLF line endings
* @remarks This is needed to avoid prettier formatting issues on generation of the files
* @param text The text to convert
* @returns The converted text
*/
const withCRLF = text => text.replace(/\n/g, "\r\n");

module.exports = { withCRLF };
2 changes: 1 addition & 1 deletion scripts/create-rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const author = argv.author || "$AUTHOR";
const description = argv.description || "$DESCRIPTION";

const rulePath = resolve(`lib/rules/${ruleName}.ts`);
const testPath = resolve(`tests/lib/rules/${ruleName}-test.ts`);
const testPath = resolve(`tests/lib/rules/${ruleName}.test.ts`);
const docsPath = resolve(`docs/rules/${ruleName}.md`);

// Validate
Expand Down
40 changes: 40 additions & 0 deletions tests/lib/rules/image-needs-alt.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { Rule } from "eslint";
import ruleTester from "./helper/ruleTester";
import rule from "../../../lib/rules/image-needs-alt";

// -----------------------------------------------------------------------------
// Tests
// -----------------------------------------------------------------------------

ruleTester.run("image-needs-alt", rule as unknown as Rule.RuleModule, {
valid: [
// Not an Image
"<div></div>",
// Valid string test
'<Image src="image.png" alt="Description of image" />',
// Valid expression test
'<Image src="image.png" alt={altText} />',
// Decorative image with empty alt
'<Image src="image.png" alt="" />'
],
invalid: [
{
// No alt attribute
code: '<Image src="image.png" />',
errors: [{ messageId: "imageNeedsAlt" }]
},
{
// Null alt attribute
code: '<Image src="image.png" alt={null} />',
errors: [{ messageId: "imageNeedsAlt" }]
},
{
// Undefined alt attribute
code: '<Image src="image.png" alt={undefined} />',
errors: [{ messageId: "imageNeedsAlt" }]
}
]
});
Loading
Loading