PR feedback

Author: rbitting

README.md

@@ -126,7 +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                                                                | ✅  |    |    |
+| [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.              | ✅  |    | 🔧 |

docs/rules/image-needs-alt.md

@@ -1,12 +1,12 @@
-# Accessibility: Image must have alt attribute with a meaningful description of the image (`@microsoft/fluentui-jsx-a11y/image-needs-alt`)
+# 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.
+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:
@@ -15,10 +15,6 @@ Examples of **incorrect** code for this rule:
 <Image src="image.png" />
 ```
 
-```jsx
-<Image src="image.png" alt="" />
-```
-
 ```jsx
 <Image src="image.png" alt={null} />
 ```
@@ -29,6 +25,10 @@ Examples of **correct** code for this rule:
 <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)

lib/rules/image-needs-alt.ts

@@ -12,7 +12,8 @@ const rule = ESLintUtils.RuleCreator.withoutDocs(
     makeLabeledControlRule({
         component: "Image",
         messageId: "imageNeedsAlt",
-        description: "Accessibility: Image must have alt attribute with a meaningful description of the image",
+        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,

lib/rules/swatchpicker-needs-labelling.ts

@@ -13,7 +13,7 @@ export default ESLintUtils.RuleCreator.withoutDocs(
         component: "SwatchPicker",
         messageId: "noUnlabeledSwatchPicker",
         description: "Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc..",
-        requiredNonEmptyProps: ["aria-label"],
+        labelProps: ["aria-label"],
         allowFieldParent: true,
         allowHtmlFor: false,
         allowLabelledBy: true,

lib/util/ruleFactory.ts

@@ -29,8 +29,8 @@ export type LabeledControlConfig = {
     description: string;
     /** Properties that are required to have a non-`null` and non-`undefined` value. @example ["alt"] */
     requiredProps?: string[];
-    /** Properties that are required to be defined and not empty. @example ["aria-label", "title", "label"] */
-    requiredNonEmptyProps?: 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. */
@@ -83,14 +83,14 @@ export function hasAccessibleLabel(
         allowTooltipParent,
         allowDescribedBy,
         allowLabeledChild,
-        requiredNonEmptyProps,
-        requiredProps
+        requiredProps,
+        labelProps
     } = config;
     const allowTextContentChild = !!config.allowTextContentChild;
 
     if (allowFieldParent && hasFieldParent(context)) return true;
-    if (requiredProps?.some(p => hasDefinedProp(opening.attributes, p))) return true;
-    if (requiredNonEmptyProps?.some(p => hasNonEmptyProp(opening.attributes, p))) return true;
+    if (requiredProps?.every(p => hasDefinedProp(node.attributes, p))) return true;
+    if (labelProps?.some(p => hasNonEmptyProp(node.attributes, p))) return true;
     if (allowWrappingLabel && isInsideLabelTag(context)) return true;
     if (allowHtmlFor && hasAssociatedLabelViaHtmlFor(opening, context)) return true;
     if (allowLabelledBy && hasAssociatedLabelViaAriaLabelledBy(opening, context)) return true;

tests/lib/rules/utils/ruleFactory.test.ts

@@ -107,7 +107,7 @@ describe("hasAccessibleLabel (unit)", () => {
     const cfg: Required<LabeledControlConfig> = {
         component: "RadioGroup",
         requiredProps: ["alt"],
-        requiredNonEmptyProps: ["label", "aria-label"],
+        labelProps: ["label", "aria-label"],
         allowFieldParent: true,
         allowHtmlFor: true,
         allowLabelledBy: true,
@@ -292,7 +292,7 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
     const baseCfg: Required<LabeledControlConfig> = {
         component: "RadioGroup",
         requiredProps: ["alt"],
-        requiredNonEmptyProps: ["label", "aria-label"],
+        labelProps: ["label", "aria-label"],
         allowFieldParent: true,
         allowHtmlFor: true,
         allowLabelledBy: true,