From 3645a88dffeeac27be76f03819d98c7501ceb663 Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Tue, 5 Nov 2024 18:15:55 +0000 Subject: [PATCH 01/10] fixed false negative on the field component --- docs/rules/field-needs-labelling.md | 33 +++----- lib/rules/field-needs-labelling.js | 10 +-- lib/util/flattenChildren.js | 21 ----- lib/util/flattenChildren.ts | 20 +++++ lib/util/hasFieldParent.js | 31 ------- lib/util/hasFieldParent.ts | 31 +++++++ lib/util/hasLabelledChildImage.js | 2 +- tests/lib/rules/utils/flattenChildren.test.ts | 73 ++++++++++++++++ tests/lib/rules/utils/hasFieldParent.test.ts | 84 +++++++++++++++++++ .../labelUtils.test.ts} | 12 +-- 10 files changed, 228 insertions(+), 89 deletions(-) delete mode 100644 lib/util/flattenChildren.js create mode 100644 lib/util/flattenChildren.ts delete mode 100644 lib/util/hasFieldParent.js create mode 100644 lib/util/hasFieldParent.ts create mode 100644 tests/lib/rules/utils/flattenChildren.test.ts create mode 100644 tests/lib/rules/utils/hasFieldParent.test.ts rename tests/lib/rules/{label-utils.test.ts => utils/labelUtils.test.ts} (84%) diff --git a/docs/rules/field-needs-labelling.md b/docs/rules/field-needs-labelling.md index 37fb792..fca690f 100644 --- a/docs/rules/field-needs-labelling.md +++ b/docs/rules/field-needs-labelling.md @@ -1,10 +1,10 @@ -# Accessibility: Field must have either label, validationMessage and hint attributes (`@microsoft/fluentui-jsx-a11y/field-needs-labelling`) +# Accessibility: Field must have label attribute (`@microsoft/fluentui-jsx-a11y/field-needs-labelling`) πŸ’Ό This rule is enabled in the βœ… `recommended` config. -Field must have `label` prop and either `validationMessage` or `hint` prop. +Field must have `label` prop. @@ -12,7 +12,6 @@ Field must have `label` prop and either `validationMessage` or `hint` prop. - Make sure that Field component has following props: - `label` - - `validationMessage` or `hint` ## Rule Details @@ -21,19 +20,13 @@ This rule aims to make Field component accessible. Examples of **incorrect** code for this rule: ```jsx - + ``` ```jsx - + ``` @@ -41,21 +34,19 @@ Examples of **incorrect** code for this rule: Examples of **correct** code for this rule: ```jsx - + + + +``` + +```jsx + ``` ```jsx - + ``` diff --git a/lib/rules/field-needs-labelling.js b/lib/rules/field-needs-labelling.js index 4db33e4..1f209c9 100644 --- a/lib/rules/field-needs-labelling.js +++ b/lib/rules/field-needs-labelling.js @@ -14,13 +14,13 @@ module.exports = { meta: { // possible error messages for the rule messages: { - noUnlabelledField: "Accessibility: Field must have either label, validationMessage and hint attributes" + noUnlabelledField: "Accessibility: Field must have label" }, // "problem" means the rule is identifying code that either will cause an error or may cause a confusing behavior: https://eslint.org/docs/latest/developer-guide/working-with-rules type: "problem", // docs for the rule docs: { - description: "Accessibility: Field must have either label, validationMessage and hint attributes", + description: "Accessibility: Field must have label", recommended: true, url: "https://www.w3.org/TR/html-aria/" // URL to the documentation page for this rule }, @@ -36,10 +36,7 @@ module.exports = { return; } - if ( - hasNonEmptyProp(node.attributes, "label", true) && - (hasNonEmptyProp(node.attributes, "validationMessage", true) || hasNonEmptyProp(node.attributes, "hint", true)) - ) { + if (hasNonEmptyProp(node.attributes, "label")) { return; } @@ -52,4 +49,3 @@ module.exports = { }; } }; - diff --git a/lib/util/flattenChildren.js b/lib/util/flattenChildren.js deleted file mode 100644 index d8ff7f3..0000000 --- a/lib/util/flattenChildren.js +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -// TODO: add comments -function flattenChildren(node) { - const flatChildren = []; - - if (node.children && node.children.length > 0) { - node.children.forEach(child => { - if (child.type === 'JSXElement' && child.children && child.children.length > 0) { - flatChildren.push(child, ...flattenChildren(child)); - } else { - flatChildren.push(child); - } - }); - } - - return flatChildren; -} - -module.exports.flattenChildren = flattenChildren; diff --git a/lib/util/flattenChildren.ts b/lib/util/flattenChildren.ts new file mode 100644 index 0000000..6bfd9ef --- /dev/null +++ b/lib/util/flattenChildren.ts @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { TSESTree } from "@typescript-eslint/types"; + +// Flatten the JSX tree structure by recursively collecting all child elements +export default function flattenChildren(node: TSESTree.JSXElement): TSESTree.JSXElement[] { + const flatChildren: TSESTree.JSXElement[] = []; + + if (node.children && node.children.length > 0) { + node.children.forEach(child => { + if (child.type === "JSXElement") { + const jsxChild = child as TSESTree.JSXElement; + flatChildren.push(jsxChild, ...flattenChildren(jsxChild)); + } + }); + } + + return flatChildren; +} diff --git a/lib/util/hasFieldParent.js b/lib/util/hasFieldParent.js deleted file mode 100644 index 223d1e4..0000000 --- a/lib/util/hasFieldParent.js +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -var elementType = require("jsx-ast-utils").elementType; - -function hasFieldParent(context) { - const ancestors = context.getAncestors(); - - if (ancestors == null || ancestors.length === 0) { - return false; - } - - var field = false; - - ancestors.forEach(item => { - if ( - item.type === "JSXElement" && - item.openingElement != null && - item.openingElement.type === "JSXOpeningElement" && - elementType(item.openingElement) === "Field" - ) { - field = true; - } - }); - - return field; -} - -module.exports = { - hasFieldParent -}; diff --git a/lib/util/hasFieldParent.ts b/lib/util/hasFieldParent.ts new file mode 100644 index 0000000..9ad97c0 --- /dev/null +++ b/lib/util/hasFieldParent.ts @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { TSESTree } from "@typescript-eslint/types"; +import { elementType } from "jsx-ast-utils"; +import { TSESLint } from "@typescript-eslint/utils"; +import { JSXOpeningElement } from "estree-jsx"; + +// Function to check if the current node has a "Field" parent JSXElement +export function hasFieldParent(context: TSESLint.RuleContext): boolean { + const ancestors: TSESTree.Node[] = context.getAncestors(); + + if (ancestors == null || ancestors.length === 0) { + return false; + } + + let field = false; + + ancestors.forEach(item => { + if ( + item.type === "JSXElement" && + item.openingElement != null && + item.openingElement.type === "JSXOpeningElement" && + elementType(item.openingElement as unknown as JSXOpeningElement) === "Field" + ) { + field = true; + } + }); + + return field; +} diff --git a/lib/util/hasLabelledChildImage.js b/lib/util/hasLabelledChildImage.js index 9b5684d..7875c15 100644 --- a/lib/util/hasLabelledChildImage.js +++ b/lib/util/hasLabelledChildImage.js @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -const { flattenChildren } = require("./flattenChildren"); +const { flattenChildren } = require("./flattenChildren.ts"); const { hasProp, getPropValue } = require("jsx-ast-utils"); const { hasNonEmptyProp } = require("./hasNonEmptyProp"); const { fluentImageComponents, imageDomNodes } = require("../applicableComponents/imageBasedComponents"); diff --git a/tests/lib/rules/utils/flattenChildren.test.ts b/tests/lib/rules/utils/flattenChildren.test.ts new file mode 100644 index 0000000..e2bf5d6 --- /dev/null +++ b/tests/lib/rules/utils/flattenChildren.test.ts @@ -0,0 +1,73 @@ +import { flattenChildren } from "../../../../lib/util/flattenChildren"; +import { TSESTree } from "@typescript-eslint/types"; + +describe("flattenChildren", () => { + test("should return an empty array when node has no children", () => { + const node: TSESTree.JSXElement = { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [] + }; + const result = flattenChildren(node); + expect(result).toEqual([]); + }); + + test("should return the same array when node has no nested JSXElement children", () => { + const node: TSESTree.JSXElement = { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [ + { type: "JSXText", value: "Hello" } as TSESTree.JSXText, + { type: "JSXExpressionContainer" } as TSESTree.JSXExpressionContainer + ] + }; + const result = flattenChildren(node); + expect(result).toEqual([]); + }); + + test("should flatten nested JSXElement children", () => { + const node: TSESTree.JSXElement = { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [ + { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [{ type: "JSXText", value: "Nested" } as TSESTree.JSXText] + } as TSESTree.JSXElement + ] + }; + const result = flattenChildren(node); + expect(result).toEqual([node.children[0], { type: "JSXText", value: "Nested" }]); + }); + + test("should handle mixed nested and non-nested JSXElement children", () => { + const node: TSESTree.JSXElement = { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [ + { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [ + { type: "JSXText", value: "Text" } as TSESTree.JSXText, + { + type: "JSXElement", + openingElement: {} as TSESTree.JSXOpeningElement, + closingElement: null, + children: [] + } as TSESTree.JSXElement + ] + } as TSESTree.JSXElement + ] + }; + const result = flattenChildren(node); + expect(result).toEqual([node.children[0], { type: "JSXText", value: "Text" }, node.children[0].children[1]]); + }); +}); diff --git a/tests/lib/rules/utils/hasFieldParent.test.ts b/tests/lib/rules/utils/hasFieldParent.test.ts new file mode 100644 index 0000000..433ac68 --- /dev/null +++ b/tests/lib/rules/utils/hasFieldParent.test.ts @@ -0,0 +1,84 @@ +import { hasFieldParent } from "../../../../lib/util/hasFieldParent"; +import { TSESTree } from "@typescript-eslint/types"; +import { TSESLint } from "@typescript-eslint/utils"; +import { elementType } from "jsx-ast-utils"; + +// Mock the elementType function to return "Field" when needed +jest.mock("jsx-ast-utils", () => ({ + elementType: jest.fn() +})); + +// Mock context to simulate ESLint's RuleContext +const createMockContext = (ancestors: TSESTree.Node[]): TSESLint.RuleContext => ({ + // Mock the required properties of RuleContext + id: "mockRule", + options: [], + settings: {}, + parserPath: "", + parserOptions: {}, + getCwd: jest.fn(), + getFilename: jest.fn(() => "mockFile.js"), + getScope: jest.fn(), + report: jest.fn(), + getAncestors: () => ancestors, + getSourceCode: jest.fn(), + getDeclaredVariables: function (node: TSESTree.Node): readonly TSESLint.Scope.Variable[] { + throw new Error("Function not implemented."); + }, + markVariableAsUsed: function (name: string): boolean { + throw new Error("Function not implemented."); + } +}); + +describe("hasFieldParent", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test("should return false when there are no ancestors", () => { + const mockContext = createMockContext([]); + const result = hasFieldParent(mockContext); + expect(result).toBe(false); + }); + + test("should return false when there are ancestors but none are JSXElements", () => { + const mockContext = createMockContext([{ type: "Literal" } as TSESTree.Literal]); + const result = hasFieldParent(mockContext); + expect(result).toBe(false); + }); + + test('should return false when none of the ancestors are "Field" elements', () => { + const mockContext = createMockContext([ + { + type: "JSXElement", + openingElement: { type: "JSXOpeningElement" } + } as TSESTree.JSXElement + ]); + (elementType as jest.Mock).mockReturnValue("NotAField"); + const result = hasFieldParent(mockContext); + expect(result).toBe(false); + }); + + test('should return true when one of the ancestors is a "Field" element', () => { + const mockContext = createMockContext([ + { + type: "JSXElement", + openingElement: { type: "JSXOpeningElement" } + } as TSESTree.JSXElement + ]); + (elementType as jest.Mock).mockReturnValue("Field"); + const result = hasFieldParent(mockContext); + expect(result).toBe(true); + }); + + test('should handle multiple ancestors with one "Field" element', () => { + const mockContext = createMockContext([ + { type: "JSXElement", openingElement: { type: "JSXOpeningElement" } } as TSESTree.JSXElement, + { type: "Literal" } as TSESTree.Literal, + { type: "JSXElement", openingElement: { type: "JSXOpeningElement" } } as TSESTree.JSXElement + ]); + (elementType as jest.Mock).mockReturnValueOnce("NotAField").mockReturnValueOnce("Field"); + const result = hasFieldParent(mockContext); + expect(result).toBe(true); + }); +}); diff --git a/tests/lib/rules/label-utils.test.ts b/tests/lib/rules/utils/labelUtils.test.ts similarity index 84% rename from tests/lib/rules/label-utils.test.ts rename to tests/lib/rules/utils/labelUtils.test.ts index 8c2017b..1d600e2 100644 --- a/tests/lib/rules/label-utils.test.ts +++ b/tests/lib/rules/utils/labelUtils.test.ts @@ -1,12 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import chai from "chai"; -import { isInsideLabelTag } from "../../../lib/util/labelUtils"; +import { isInsideLabelTag } from "../../../../lib/util/labelUtils"; -const assert: Chai.AssertStatic = chai.assert; - -console.log(assert); describe("isInsideLabelTag", function () { it("should return true when nested within a Label tag", function () { const context = { @@ -21,7 +17,7 @@ describe("isInsideLabelTag", function () { const result = isInsideLabelTag(context); - assert.isTrue(result); + expect(result).toBe(true); }); it("should return true when nested within a label tag (case-insensitive)", function () { @@ -37,7 +33,7 @@ describe("isInsideLabelTag", function () { const result = isInsideLabelTag(context); - assert.isTrue(result); + expect(result).toBe(true); }); it("should return false when not nested within a Label tag", function () { @@ -53,6 +49,6 @@ describe("isInsideLabelTag", function () { const result = isInsideLabelTag(context); - assert.isFalse(result); + expect(result).toBe(false); }); }); From 458f8a36ba38a44b48d0c843ba42284d1994ba9d Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Tue, 5 Nov 2024 18:23:32 +0000 Subject: [PATCH 02/10] added unit tests --- README.md | 2 +- docs/rules/field-needs-labelling.md | 2 +- ...-labelling.js => field-needs-labelling.ts} | 13 +- lib/rules/tablist-and-tabs-need-labelling.js | 76 ------ lib/rules/tablist-and-tabs-need-labelling.ts | 77 ++++++ ...sual-label-better-than-aria-suggestion.ts} | 29 ++- lib/util/flattenChildren.ts | 6 +- lib/util/hasFieldParent.ts | 4 +- lib/util/hasLabelledChildImage.js | 40 --- lib/util/hasLabelledChildImage.ts | 74 ++++++ lib/util/hasNonEmptyProp.js | 33 --- lib/util/hasNonEmptyProp.ts | 53 ++++ ...ContentChild.js => hasTextContentChild.ts} | 14 +- lib/util/labelUtils.js | 153 ------------ lib/util/labelUtils.ts | 219 +++++++++++++++++ ...lling.js => field-needs-labelling.test.ts} | 30 +-- ...> tablist-and-tabs-need-labelling.test.ts} | 38 +-- tests/lib/rules/utils/flattenChildren.test.ts | 93 +++---- tests/lib/rules/utils/hasFieldParent.test.ts | 3 + tests/lib/rules/utils/hasNonEmptyProp.test.ts | 97 ++++++++ .../rules/utils/hasTextContentChild.test.ts | 45 ++++ .../lib/rules/utils/hasTooltipParent.test.ts | 3 + tests/lib/rules/utils/labelUtils.test.ts | 229 ++++++++++++++---- ...label-better-than-aria-suggestion.test.ts} | 26 +- tsconfig.json | 3 +- 25 files changed, 863 insertions(+), 499 deletions(-) rename lib/rules/{field-needs-labelling.js => field-needs-labelling.ts} (86%) delete mode 100644 lib/rules/tablist-and-tabs-need-labelling.js create mode 100644 lib/rules/tablist-and-tabs-need-labelling.ts rename lib/rules/{visual-label-better-than-aria-suggestion.js => visual-label-better-than-aria-suggestion.ts} (67%) delete mode 100644 lib/util/hasLabelledChildImage.js create mode 100644 lib/util/hasLabelledChildImage.ts delete mode 100644 lib/util/hasNonEmptyProp.js create mode 100644 lib/util/hasNonEmptyProp.ts rename lib/util/{hasTextContentChild.js => hasTextContentChild.ts} (74%) delete mode 100644 lib/util/labelUtils.js create mode 100644 lib/util/labelUtils.ts rename tests/lib/rules/{field-needs-labelling.js => field-needs-labelling.test.ts} (54%) rename tests/lib/rules/{tablist-and-tabs-need-labelling.js => tablist-and-tabs-need-labelling.test.ts} (65%) create mode 100644 tests/lib/rules/utils/hasNonEmptyProp.test.ts create mode 100644 tests/lib/rules/utils/hasTextContentChild.test.ts rename tests/lib/rules/{visual-label-better-than-aria-suggestion.js => visual-label-better-than-aria-suggestion.test.ts} (73%) diff --git a/README.md b/README.md index aecb184..571fdfa 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ Any use of third-party trademarks or logos are subject to those third-party's po | [dialogbody-needs-title-content-and-actions](docs/rules/dialogbody-needs-title-content-and-actions.md) | A DialogBody should have a header(DialogTitle), content(DialogContent), and footer(DialogActions) | βœ… | | | | [dialogsurface-needs-aria](docs/rules/dialogsurface-needs-aria.md) | DialogueSurface need accessible labelling: aria-describedby on DialogueSurface and aria-label or aria-labelledby(if DialogueTitle is missing) | βœ… | | | | [dropdown-needs-labelling](docs/rules/dropdown-needs-labelling.md) | Accessibility: Dropdown menu must have an id and it needs to be linked via htmlFor of a Label | βœ… | | | -| [field-needs-labelling](docs/rules/field-needs-labelling.md) | Accessibility: Field must have either label, validationMessage and hint attributes | βœ… | | | +| [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 | βœ… | | | | [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. | βœ… | | πŸ”§ | diff --git a/docs/rules/field-needs-labelling.md b/docs/rules/field-needs-labelling.md index fca690f..37332ae 100644 --- a/docs/rules/field-needs-labelling.md +++ b/docs/rules/field-needs-labelling.md @@ -1,4 +1,4 @@ -# Accessibility: Field must have label attribute (`@microsoft/fluentui-jsx-a11y/field-needs-labelling`) +# Accessibility: Field must have label (`@microsoft/fluentui-jsx-a11y/field-needs-labelling`) πŸ’Ό This rule is enabled in the βœ… `recommended` config. diff --git a/lib/rules/field-needs-labelling.js b/lib/rules/field-needs-labelling.ts similarity index 86% rename from lib/rules/field-needs-labelling.js rename to lib/rules/field-needs-labelling.ts index 1f209c9..cc0536b 100644 --- a/lib/rules/field-needs-labelling.js +++ b/lib/rules/field-needs-labelling.ts @@ -5,12 +5,14 @@ const { hasNonEmptyProp } = require("../util/hasNonEmptyProp"); const elementType = require("jsx-ast-utils").elementType; +import { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ -module.exports = { +const rule = ESLintUtils.RuleCreator.withoutDocs({ + defaultOptions: [], meta: { // possible error messages for the rule messages: { @@ -21,16 +23,17 @@ module.exports = { // docs for the rule docs: { description: "Accessibility: Field must have label", - recommended: true, + recommended: "strict", url: "https://www.w3.org/TR/html-aria/" // URL to the documentation page for this rule }, schema: [] }, + // create (function) returns an object with methods that ESLint calls to β€œvisit” nodes while traversing the abstract syntax tree create(context) { return { // visitor functions for different types of nodes - JSXOpeningElement(node) { + JSXOpeningElement(node: TSESTree.JSXOpeningElement) { // if it is not a Spinner, return if (elementType(node) !== "Field") { return; @@ -48,4 +51,6 @@ module.exports = { } }; } -}; +}); + +export default rule; diff --git a/lib/rules/tablist-and-tabs-need-labelling.js b/lib/rules/tablist-and-tabs-need-labelling.js deleted file mode 100644 index aef89ba..0000000 --- a/lib/rules/tablist-and-tabs-need-labelling.js +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -"use strict"; - -//------------------------------------------------------------------------------ -// Rule Definition -//------------------------------------------------------------------------------ - -/** @type {import('eslint').Rule.RuleModule} */ -module.exports = { - meta: { - type: 'problem', - docs: { - description: 'This rule aims to ensure that Tabs with icons but no text labels have an accessible name and that Tablist is properly labeled.', - recommended: true, - url: 'https://www.w3.org/WAI/ARIA/apg/patterns/tabs/', // URL to the documentation page for this rule - }, - fixable: null, - schema: [], - messages: { - missingTabLabel: 'Accessibility: Tab elements must have an aria-label attribute is there is no visiable text content', - missingTablistLabel: 'Accessibility: Tablist must have an accessible label' - }, - }, - - create(context) { - const { hasTextContentChild } = require('../util/hasTextContentChild'); - const { hasNonEmptyProp } = require('../util/hasNonEmptyProp'); - const { hasAssociatedLabelViaAriaLabelledBy } = require('../util/labelUtils'); - - var elementType = require("jsx-ast-utils").elementType; - - return { - - // visitor functions for different types of nodes - JSXOpeningElement(node) { - const elementTypeValue = elementType(node); - - // if it is not a Tablist or Tab, return - if (elementTypeValue !== 'Tablist' && elementTypeValue !== 'Tab') { - return; - } - - // Check for Tablist elements - if (elementTypeValue === "Tablist") { - if ( - // if the Tablist has a label, if the Tablist has an associated label, return - hasNonEmptyProp(node.attributes, 'aria-label') || //aria-label - hasAssociatedLabelViaAriaLabelledBy(node, context) // aria-labelledby - ) { - return; - } - context.report({ - node, - messageId: 'missingTablistLabel' - }); - } - - // Check for Tab elements - if (elementTypeValue === 'Tab') { - if ( - hasTextContentChild(node.parent) || // text content - hasNonEmptyProp(node.attributes, 'aria-label') // aria-label - ) { - return; - } - context.report({ - node, - messageId: 'missingTabLabel' - }); - } - } - }; - }, -}; diff --git a/lib/rules/tablist-and-tabs-need-labelling.ts b/lib/rules/tablist-and-tabs-need-labelling.ts new file mode 100644 index 0000000..3545fc0 --- /dev/null +++ b/lib/rules/tablist-and-tabs-need-labelling.ts @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; +import { hasTextContentChild } from "../util/hasTextContentChild"; +import { hasNonEmptyProp } from "../util/hasNonEmptyProp"; +import { hasAssociatedLabelViaAriaLabelledBy } from "../util/labelUtils"; +import { elementType } from "jsx-ast-utils"; +import { JSXOpeningElement } from "estree-jsx"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const rule = ESLintUtils.RuleCreator.withoutDocs({ + defaultOptions: [], + meta: { + type: "problem", + docs: { + description: + "This rule aims to ensure that Tabs with icons but no text labels have an accessible name and that Tablist is properly labeled.", + recommended: "strict", + url: "https://www.w3.org/WAI/ARIA/apg/patterns/tabs/" // URL to the documentation page for this rule + }, + fixable: undefined, + schema: [], + messages: { + missingTabLabel: "Accessibility: Tab elements must have an aria-label attribute is there is no visiable text content", + missingTablistLabel: "Accessibility: Tablist must have an accessible label" + } + }, + + create(context) { + return { + // visitor functions for different types of nodes + JSXOpeningElement(node: TSESTree.JSXOpeningElement) { + const elementTypeValue = elementType(node as unknown as JSXOpeningElement); + + // if it is not a Tablist or Tab, return + if (elementTypeValue !== "Tablist" && elementTypeValue !== "Tab") { + return; + } + + // Check for Tablist elements + if (elementTypeValue === "Tablist") { + if ( + // if the Tablist has a label, if the Tablist has an associated label, return + hasNonEmptyProp(node.attributes, "aria-label") || //aria-label + hasAssociatedLabelViaAriaLabelledBy(node, context) // aria-labelledby + ) { + return; + } + context.report({ + node, + messageId: "missingTablistLabel" + }); + } + + // Check for Tab elements + if (elementTypeValue === "Tab") { + if ( + hasTextContentChild(node.parent as unknown as TSESTree.JSXElement) || // text content + hasNonEmptyProp(node.attributes, "aria-label") // aria-label + ) { + return; + } + context.report({ + node, + messageId: "missingTabLabel" + }); + } + } + }; + } +}); + +export default rule; diff --git a/lib/rules/visual-label-better-than-aria-suggestion.js b/lib/rules/visual-label-better-than-aria-suggestion.ts similarity index 67% rename from lib/rules/visual-label-better-than-aria-suggestion.js rename to lib/rules/visual-label-better-than-aria-suggestion.ts index 1d167e7..4f3e000 100644 --- a/lib/rules/visual-label-better-than-aria-suggestion.js +++ b/lib/rules/visual-label-better-than-aria-suggestion.ts @@ -1,18 +1,18 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -"use strict"; - -var elementType = require("jsx-ast-utils").elementType; -const { hasNonEmptyProp } = require("../util/hasNonEmptyProp"); -const { applicableComponents } = require("../applicableComponents/buttonBasedComponents"); +import { hasNonEmptyProp } from "../util/hasNonEmptyProp"; +import { applicableComponents } from "../applicableComponents/buttonBasedComponents"; +import { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; +import { elementType } from "jsx-ast-utils"; +import { JSXOpeningElement } from "estree-jsx"; //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ -/** @type {import('eslint').Rule.RuleModule} */ -module.exports = { +const rule = ESLintUtils.RuleCreator.withoutDocs({ + defaultOptions: [], meta: { // possible warning messages for the lint rule messages: { @@ -21,19 +21,20 @@ module.exports = { type: "suggestion", // `problem`, `suggestion`, or `layout` docs: { description: "Visual label is better than an aria-label", - recommended: true, - url: null // URL to the documentation page for this rule + recommended: "strict", + url: undefined // URL to the documentation page for this rule }, - fixable: null, // Or `code` or `whitespace` + fixable: undefined, // Or `code` or `whitespace` schema: [] // Add a schema if the rule has options }, + // create (function) returns an object with methods that ESLint calls to β€œvisit” nodes while traversing the abstract syntax tree create(context) { return { // visitor functions for different types of nodes - JSXOpeningElement(node) { + JSXOpeningElement(node: TSESTree.JSXOpeningElement) { // if it is not a listed component, return - if (!applicableComponents.includes(elementType(node))) { + if (!applicableComponents.includes(elementType(node as unknown as JSXOpeningElement))) { return; } @@ -47,4 +48,6 @@ module.exports = { } }; } -}; +}); + +export default rule; diff --git a/lib/util/flattenChildren.ts b/lib/util/flattenChildren.ts index 6bfd9ef..c8b285a 100644 --- a/lib/util/flattenChildren.ts +++ b/lib/util/flattenChildren.ts @@ -4,7 +4,7 @@ import { TSESTree } from "@typescript-eslint/types"; // Flatten the JSX tree structure by recursively collecting all child elements -export default function flattenChildren(node: TSESTree.JSXElement): TSESTree.JSXElement[] { +const flattenChildren = (node: TSESTree.JSXElement): TSESTree.JSXElement[] => { const flatChildren: TSESTree.JSXElement[] = []; if (node.children && node.children.length > 0) { @@ -17,4 +17,6 @@ export default function flattenChildren(node: TSESTree.JSXElement): TSESTree.JSX } return flatChildren; -} +}; + +export { flattenChildren }; diff --git a/lib/util/hasFieldParent.ts b/lib/util/hasFieldParent.ts index 9ad97c0..dd4605c 100644 --- a/lib/util/hasFieldParent.ts +++ b/lib/util/hasFieldParent.ts @@ -7,7 +7,7 @@ import { TSESLint } from "@typescript-eslint/utils"; import { JSXOpeningElement } from "estree-jsx"; // Function to check if the current node has a "Field" parent JSXElement -export function hasFieldParent(context: TSESLint.RuleContext): boolean { +export const hasFieldParent = (context: TSESLint.RuleContext): boolean => { const ancestors: TSESTree.Node[] = context.getAncestors(); if (ancestors == null || ancestors.length === 0) { @@ -28,4 +28,4 @@ export function hasFieldParent(context: TSESLint.RuleContext) }); return field; -} +}; diff --git a/lib/util/hasLabelledChildImage.js b/lib/util/hasLabelledChildImage.js deleted file mode 100644 index 7875c15..0000000 --- a/lib/util/hasLabelledChildImage.js +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -const { flattenChildren } = require("./flattenChildren.ts"); -const { hasProp, getPropValue } = require("jsx-ast-utils"); -const { hasNonEmptyProp } = require("./hasNonEmptyProp"); -const { fluentImageComponents, imageDomNodes } = require("../applicableComponents/imageBasedComponents"); - -const mergedImageComponents = [...fluentImageComponents, ...imageDomNodes]; - -/** - * hasLabelledChildImage - determines if a component has text content as a child e.g. abc - * @param {*} node JSXElement - * @returns boolean - */ -function hasLabelledChildImage(node) { - // no children - if (node.children == null || node.children == undefined || node.children.length === 0) { - return false; - } - - // Check if there is an accessible image - const hasAccessibleImage = flattenChildren(node).some(child => { - if (child.type === "JSXElement" && mergedImageComponents.includes(child.openingElement.name.name)) { - return hasProp(child.openingElement.attributes, "aria-hidden") || getPropValue(child.openingElement.attributes, "alt") - ? false - : hasNonEmptyProp(child.openingElement.attributes, "title") || - hasNonEmptyProp(child.openingElement.attributes, "alt") || - hasNonEmptyProp(child.openingElement.attributes, "aria-label") || - hasNonEmptyProp(child.openingElement.attributes, "aria-labelledby"); - } - return false; - }); - - return hasAccessibleImage; -} - -module.exports = { - hasLabelledChildImage -}; diff --git a/lib/util/hasLabelledChildImage.ts b/lib/util/hasLabelledChildImage.ts new file mode 100644 index 0000000..a5f71d5 --- /dev/null +++ b/lib/util/hasLabelledChildImage.ts @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { flattenChildren } from "./flattenChildren"; +import { TSESTree } from "@typescript-eslint/types"; +import { hasProp, getPropValue, getProp } from "jsx-ast-utils"; +import { hasNonEmptyProp } from "./hasNonEmptyProp"; +import { fluentImageComponents, imageDomNodes } from "../applicableComponents/imageBasedComponents"; +import { JSXOpeningElement } from "estree-jsx"; + +const mergedImageComponents = [...fluentImageComponents, ...imageDomNodes]; + +/** + * Checks if a JSX element name is a JSXIdentifier and matches a component name. + * @param name JSXTagNameExpression + * @returns boolean + */ +const isJSXIdentifierWithName = (name: TSESTree.JSXTagNameExpression, validNames: string[]): boolean => { + return name.type === "JSXIdentifier" && validNames.includes(name.name); +}; + +/** + * Determines if a component has an accessible image as a child. + * @param {*} node JSXElement + * @returns boolean + */ +const hasLabelledChildImage = (node: TSESTree.JSXElement): boolean => { + if (!node.children || node.children.length === 0) { + return false; + } + + return flattenChildren(node).some(child => { + if (child.type === "JSXElement" && isJSXIdentifierWithName(child.openingElement.name, mergedImageComponents)) { + const attributes = child.openingElement.attributes; + return !isImageHidden(attributes) && hasAccessibilityAttributes(attributes); + } + return false; + }); +}; + +/** + * Checks if an image element has any of the attributes indicating it is accessible. + * @param {*} attributes JSX attributes of the image element + * @returns boolean + */ +const hasAccessibilityAttributes = (attributes: TSESTree.JSXOpeningElement["attributes"]): boolean => { + return ( + hasNonEmptyProp(attributes, "title") || + hasNonEmptyProp(attributes, "alt") || + hasNonEmptyProp(attributes, "aria-label") || + hasNonEmptyProp(attributes, "aria-labelledby") + ); +}; + +/** + * Checks if an image element is marked as hidden using `aria-hidden` or has an empty `alt` attribute. + * @param {*} attributes JSX attributes of the image element + * @returns boolean + */ +const isImageHidden = (attributes: TSESTree.JSXOpeningElement["attributes"]): boolean => { + if (hasProp(attributes as unknown as JSXOpeningElement["attributes"], "aria-hidden")) { + return true; + } + + const altProp = getProp(attributes as unknown as JSXOpeningElement["attributes"], "alt"); + if (altProp) { + const altValue = getPropValue(altProp); + return !altValue; // Returns true if `altValue` is falsy (e.g., empty string, null, or undefined) + } + + return true; // If `alt` is not present, consider the image hidden +}; + +export { hasLabelledChildImage, isImageHidden, hasAccessibilityAttributes, isJSXIdentifierWithName }; diff --git a/lib/util/hasNonEmptyProp.js b/lib/util/hasNonEmptyProp.js deleted file mode 100644 index 2d35771..0000000 --- a/lib/util/hasNonEmptyProp.js +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -var hasProp = require("jsx-ast-utils").hasProp; -var getPropValue = require("jsx-ast-utils").getPropValue; -var getProp = require("jsx-ast-utils").getProp; - -/** - * Determines if the prop exists and has a non-empty value. - * @param {*} attributes - * @param {*} name - * @returns boolean - */ -function hasNonEmptyProp(attributes, name) { - if (!hasProp(attributes, name)) { - return false; - } - - const propValue = getPropValue(getProp(attributes, name)); - - /** - * getPropValue internally normalizes "true", "false" to boolean values. - * So it is sufficent to check if the prop exists and return. - */ - if (typeof propValue === "boolean" || typeof propValue === "number") { - return true; - } - - return propValue.trim().length > 0; -} - -module.exports.hasNonEmptyProp = hasNonEmptyProp; - diff --git a/lib/util/hasNonEmptyProp.ts b/lib/util/hasNonEmptyProp.ts new file mode 100644 index 0000000..83a2fb7 --- /dev/null +++ b/lib/util/hasNonEmptyProp.ts @@ -0,0 +1,53 @@ +// 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 prop exists and has a non-empty value. + * @param {*} attributes + * @param {*} name + * @returns boolean + */ +const hasNonEmptyProp = (attributes: TSESTree.JSXOpeningElement["attributes"], name: string): boolean => { + if (!hasProp(attributes as unknown as JSXOpeningElement["attributes"], name)) { + return false; + } + + const prop = getProp(attributes as unknown as JSXOpeningElement["attributes"], name); + + // Safely get the value of the prop, handling potential undefined or null values + const propValue = prop ? getPropValue(prop) : undefined; + + // Check for various types that `getPropValue` could return + if (propValue === null || propValue === undefined) { + return false; + } + + if (typeof propValue === "boolean" || typeof propValue === "number") { + // Booleans and numbers are considered non-empty if they exist + return true; + } + + if (typeof propValue === "string") { + // For strings, check if it is non-empty + return propValue.trim().length > 0; + } + + // Handle other potential types (e.g., arrays, objects) + if (Array.isArray(propValue)) { + return propValue.length > 0; + } + + if (typeof propValue === "object") { + // Objects are considered non-empty if they have properties + return Object.keys(propValue).length > 0; + } + + // If the type is not handled, return false as a fallback + return false; +}; + +export { hasNonEmptyProp }; diff --git a/lib/util/hasTextContentChild.js b/lib/util/hasTextContentChild.ts similarity index 74% rename from lib/util/hasTextContentChild.js rename to lib/util/hasTextContentChild.ts index 70299ed..5f68401 100644 --- a/lib/util/hasTextContentChild.js +++ b/lib/util/hasTextContentChild.ts @@ -1,13 +1,19 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import { TSESTree } from "@typescript-eslint/types"; + /** * hasTextContentChild - determines if a component has text content as a child e.g. * @param {*} node JSXElement * @returns boolean */ -function hasTextContentChild(node) { +const hasTextContentChild = (node?: TSESTree.JSXElement) => { // no children + if (!node) { + return false; + } + if (node.children == null || node.children == undefined || node.children.length === 0) { return false; } @@ -17,8 +23,6 @@ function hasTextContentChild(node) { }); return result.length !== 0; -} - -module.exports = { - hasTextContentChild }; + +export { hasTextContentChild }; diff --git a/lib/util/labelUtils.js b/lib/util/labelUtils.js deleted file mode 100644 index f3179b6..0000000 --- a/lib/util/labelUtils.js +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -var elementType = require("jsx-ast-utils").elementType; -var getPropValue = require("jsx-ast-utils").getPropValue; -var getProp = require("jsx-ast-utils").getProp; -const { hasNonEmptyProp } = require("./hasNonEmptyProp"); - -/** - * Checks if the element is nested within a Label tag. - * e.g. - * - * @param {*} context - * @returns - */ -function isInsideLabelTag(context) { - return context - .getAncestors() - .some( - node => - node.type === "JSXElement" && (elementType(node.openingElement) === "Label" || elementType(node.openingElement) === "label") - ); -} - -/** - * Checks if there is a Label component inside the source code with a htmlFor attribute matching that of the id parameter. - * e.g. - * id=parameter, - * @param {*} idValue - * @param {*} context - * @returns boolean for match found or not. - */ -function hasLabelWithHtmlForId(idValue, context) { - if (idValue === "") { - return false; - } - const sourceCode = context.getSourceCode(); - const regex = /]*htmlFor[^>]*=[^>]*[{"|{'|"|']([^>'"}]*)['|"|'}|"}][^>]*>/gim; - const matches = regex.exec(sourceCode.text); - return !!matches && matches.some(match => match === idValue); -} - -/** - * Checks if there is a Label component inside the source code with an id matching that of the id parameter. - * e.g. - * id=parameter, - * @param {*} idValue value of the props id e.g. - * - * - * @param {*} openingElement - * @param {*} context - * @param {*} ariaAttribute - * @returns boolean for match found or not. - */ -function hasAssociatedAriaText(openingElement, context, ariaAttribute) { - const hasAssociatedAriaText = hasNonEmptyProp(openingElement.attributes, ariaAttribute); - const idValue = getPropValue(getProp(openingElement.attributes, ariaAttribute)); - let hasHtmlId = false; - if (idValue) { - const sourceCode = context.getSourceCode(); - - const regex = /<(\w+)[^>]*id\s*=\s*["']([^"']*)["'][^>]*>/gi; - let match; - const ids = []; - - while ((match = regex.exec(sourceCode.text)) !== null) { - ids.push(match[2]); - } - hasHtmlId = ids.some(id => id === idValue); - } - - return hasAssociatedAriaText && hasHtmlId; -} - -module.exports = { - isInsideLabelTag, - hasLabelWithHtmlForId, - hasLabelWithHtmlId, - hasAssociatedLabelViaAriaLabelledBy, - hasAssociatedLabelViaHtmlFor, - hasAssociatedLabelViaAriaDescribedby, - hasAssociatedAriaText -}; diff --git a/lib/util/labelUtils.ts b/lib/util/labelUtils.ts new file mode 100644 index 0000000..8954574 --- /dev/null +++ b/lib/util/labelUtils.ts @@ -0,0 +1,219 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { elementType } from "jsx-ast-utils"; +import { getPropValue } from "jsx-ast-utils"; +import { getProp } from "jsx-ast-utils"; +import { hasNonEmptyProp } from "./hasNonEmptyProp"; +import { TSESLint } from "@typescript-eslint/utils"; // Assuming context comes from TSESLint +import { JSXOpeningElement } from "estree-jsx"; +import { TSESTree } from "@typescript-eslint/utils"; + +/** + * Checks if the element is nested within a Label tag. + * e.g. + * + * @param {*} context + * @returns + */ +const isInsideLabelTag = (context: TSESLint.RuleContext): boolean => { + return context.getAncestors().some(node => { + if (node.type !== "JSXElement") return false; + const tagName = elementType(node.openingElement as unknown as JSXOpeningElement); + return tagName.toLowerCase() === "label"; + }); +}; + +/** + * Checks if there is a Label component inside the source code with a htmlFor attribute matching that of the id parameter. + * e.g. + * id=parameter, + * @param {*} idValue + * @param {*} context + * @returns boolean for match found or not. + */ +const hasLabelWithHtmlForId = (idValue: string, context: TSESLint.RuleContext): boolean => { + if (idValue === "") { + return false; + } + const sourceCode = context.getSourceCode(); + const regex = /]*htmlFor[^>]*=[^>]*[{"|{'|"|']([^>'"}]*)['|"|'}|"}][^>]*>/gim; + const matches = regex.exec(sourceCode.text); + return !!matches && matches.some(match => match === idValue); +}; + +/** + * Checks if there is a Label component inside the source code with an id matching that of the id parameter. + * e.g. + * id=parameter, + * @param {*} idValue value of the props id e.g. + * + * + * @param {*} openingElement + * @param {*} context + * @param {*} ariaAttribute + * @returns boolean for match found or not. + */ +const hasAssociatedAriaText = ( + openingElement: TSESTree.JSXOpeningElement, + context: TSESLint.RuleContext, + ariaAttribute: string +) => { + const hasAssociatedAriaText = hasNonEmptyProp(openingElement.attributes, ariaAttribute); + + const prop = getProp(openingElement.attributes as unknown as JSXOpeningElement["attributes"], ariaAttribute); + + const idValue = prop ? getPropValue(prop) : undefined; + + let hasHtmlId = false; + if (idValue) { + const sourceCode = context.getSourceCode(); + + const regex = /<(\w+)[^>]*id\s*=\s*["']([^"']*)["'][^>]*>/gi; + let match; + const ids = []; + + while ((match = regex.exec(sourceCode.text)) !== null) { + ids.push(match[2]); + } + hasHtmlId = ids.some(id => id === idValue); + } + + return hasAssociatedAriaText && hasHtmlId; +}; + +export { + isInsideLabelTag, + hasLabelWithHtmlForId, + hasLabelWithHtmlId, + hasAssociatedLabelViaAriaLabelledBy, + hasAssociatedLabelViaHtmlFor, + hasAssociatedLabelViaAriaDescribedby, + hasAssociatedAriaText, + hasOtherElementWithHtmlId +}; diff --git a/tests/lib/rules/field-needs-labelling.js b/tests/lib/rules/field-needs-labelling.test.ts similarity index 54% rename from tests/lib/rules/field-needs-labelling.js rename to tests/lib/rules/field-needs-labelling.test.ts index 56fb68e..c407efa 100644 --- a/tests/lib/rules/field-needs-labelling.js +++ b/tests/lib/rules/field-needs-labelling.test.ts @@ -6,22 +6,17 @@ //------------------------------------------------------------------------------ // Requirements //------------------------------------------------------------------------------ - -const rule = require("../../../lib/rules/field-needs-labelling"), - RuleTester = require("eslint").RuleTester; +import { Rule } from "eslint"; +import ruleTester from "./helper/ruleTester"; +import rule from "../../../lib/rules/field-needs-labelling"; //------------------------------------------------------------------------------ // Tests //------------------------------------------------------------------------------ -const ruleTester = new RuleTester(); -ruleTester.run("field-needs-labelling", rule, { +ruleTester.run("field-needs-labelling", rule as unknown as Rule.RuleModule, { valid: [ - ` + ` `, ` - - `, + code: ``, errors: [{ messageId: "noUnlabelledField" }] }, { - code: ` - - `, + code: ``, errors: [{ messageId: "noUnlabelledField" }] } ] }); - diff --git a/tests/lib/rules/tablist-and-tabs-need-labelling.js b/tests/lib/rules/tablist-and-tabs-need-labelling.test.ts similarity index 65% rename from tests/lib/rules/tablist-and-tabs-need-labelling.js rename to tests/lib/rules/tablist-and-tabs-need-labelling.test.ts index ecb8ed0..8e72538 100644 --- a/tests/lib/rules/tablist-and-tabs-need-labelling.js +++ b/tests/lib/rules/tablist-and-tabs-need-labelling.test.ts @@ -1,45 +1,33 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -"use strict"; +import { Rule } from "eslint"; +import ruleTester from "./helper/ruleTester"; +import rule from "../../../lib/rules/tablist-and-tabs-need-labelling"; //------------------------------------------------------------------------------ // Requirements //------------------------------------------------------------------------------ -const rule = require("../../../lib/rules/tablist-and-tabs-need-labelling"), - RuleTester = require("eslint").RuleTester; - -RuleTester.setDefaultConfig({ - parserOptions: { - ecmaVersion: 6, - ecmaFeatures: { - jsx: true - } - } -}); - //------------------------------------------------------------------------------ // Tests //------------------------------------------------------------------------------ - -const ruleTester = new RuleTester(); -ruleTester.run("tablist-and-tabs-need-labelling", rule, { +ruleTester.run("tablist-and-tabs-need-labelling", rule as unknown as Rule.RuleModule, { valid: [ // Valid cases for Tablist 'Settings Tab', 'Settings Tab', - + // Valid cases '} aria-label="Settings" />', - '}>Settings', - 'Settings', + "}>Settings", + "Settings" ], invalid: [ // Invalid cases for Tablist { - code: 'Settings Tab', + code: "Settings Tab", errors: [{ messageId: "missingTablistLabel" }] }, { @@ -47,18 +35,18 @@ ruleTester.run("tablist-and-tabs-need-labelling", rule, { errors: [{ messageId: "missingTablistLabel" }] }, { - code: 'Settings Tab', + code: "Settings Tab", errors: [{ messageId: "missingTablistLabel" }] }, - + // Invalid cases for Tab { - code: '} />', + code: "} />", errors: [{ messageId: "missingTabLabel" }] }, { - code: '}>', + code: "}>", errors: [{ messageId: "missingTabLabel" }] - }, + } ] }); diff --git a/tests/lib/rules/utils/flattenChildren.test.ts b/tests/lib/rules/utils/flattenChildren.test.ts index e2bf5d6..6a42b3d 100644 --- a/tests/lib/rules/utils/flattenChildren.test.ts +++ b/tests/lib/rules/utils/flattenChildren.test.ts @@ -1,73 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + import { flattenChildren } from "../../../../lib/util/flattenChildren"; import { TSESTree } from "@typescript-eslint/types"; describe("flattenChildren", () => { - test("should return an empty array when node has no children", () => { + it("should return an empty array when there are no children", () => { const node: TSESTree.JSXElement = { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, children: [] - }; - const result = flattenChildren(node); - expect(result).toEqual([]); + } as any; + expect(flattenChildren(node)).toEqual([]); }); - test("should return the same array when node has no nested JSXElement children", () => { + it("should return direct children when there are no nested children", () => { + const child1: TSESTree.JSXElement = { children: [], type: "JSXElement" } as any; + const child2: TSESTree.JSXElement = { children: [], type: "JSXElement" } as any; const node: TSESTree.JSXElement = { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, - children: [ - { type: "JSXText", value: "Hello" } as TSESTree.JSXText, - { type: "JSXExpressionContainer" } as TSESTree.JSXExpressionContainer - ] - }; - const result = flattenChildren(node); - expect(result).toEqual([]); + children: [child1, child2] + } as any; + + expect(flattenChildren(node)).toEqual([child1, child2]); }); - test("should flatten nested JSXElement children", () => { - const node: TSESTree.JSXElement = { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, - children: [ - { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, - children: [{ type: "JSXText", value: "Nested" } as TSESTree.JSXText] - } as TSESTree.JSXElement - ] - }; - const result = flattenChildren(node); - expect(result).toEqual([node.children[0], { type: "JSXText", value: "Nested" }]); + it("should return a flattened array of children with nested JSXElements", () => { + const nestedChild: TSESTree.JSXElement = { children: [], type: "JSXElement" } as any; + const child: TSESTree.JSXElement = { children: [nestedChild], type: "JSXElement" } as any; + const root: TSESTree.JSXElement = { children: [child], type: "JSXElement" } as any; + + expect(flattenChildren(root)).toEqual([child, nestedChild]); }); - test("should handle mixed nested and non-nested JSXElement children", () => { - const node: TSESTree.JSXElement = { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, - children: [ - { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, - children: [ - { type: "JSXText", value: "Text" } as TSESTree.JSXText, - { - type: "JSXElement", - openingElement: {} as TSESTree.JSXOpeningElement, - closingElement: null, - children: [] - } as TSESTree.JSXElement - ] - } as TSESTree.JSXElement - ] - }; - const result = flattenChildren(node); - expect(result).toEqual([node.children[0], { type: "JSXText", value: "Text" }, node.children[0].children[1]]); + it("should ignore non-JSXElement children", () => { + const child: TSESTree.JSXElement = { children: [], type: "JSXElement" } as any; + const nonJSXChild = { type: "JSXText", value: "Hello" } as any; + const root: TSESTree.JSXElement = { children: [child, nonJSXChild], type: "JSXElement" } as any; + + expect(flattenChildren(root)).toEqual([child]); + }); + + it("should handle complex nesting of JSXElements", () => { + const grandchild1: TSESTree.JSXElement = { children: [], type: "JSXElement" } as any; + const grandchild2: TSESTree.JSXElement = { children: [], type: "JSXElement" } as any; + const child1: TSESTree.JSXElement = { children: [grandchild1], type: "JSXElement" } as any; + const child2: TSESTree.JSXElement = { children: [grandchild2], type: "JSXElement" } as any; + const root: TSESTree.JSXElement = { children: [child1, child2], type: "JSXElement" } as any; + + expect(flattenChildren(root)).toEqual([child1, grandchild1, child2, grandchild2]); }); }); diff --git a/tests/lib/rules/utils/hasFieldParent.test.ts b/tests/lib/rules/utils/hasFieldParent.test.ts index 433ac68..5e5d06c 100644 --- a/tests/lib/rules/utils/hasFieldParent.test.ts +++ b/tests/lib/rules/utils/hasFieldParent.test.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + import { hasFieldParent } from "../../../../lib/util/hasFieldParent"; import { TSESTree } from "@typescript-eslint/types"; import { TSESLint } from "@typescript-eslint/utils"; diff --git a/tests/lib/rules/utils/hasNonEmptyProp.test.ts b/tests/lib/rules/utils/hasNonEmptyProp.test.ts new file mode 100644 index 0000000..b55a40e --- /dev/null +++ b/tests/lib/rules/utils/hasNonEmptyProp.test.ts @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { hasNonEmptyProp } from "../../../../lib/util/hasNonEmptyProp"; +import { TSESTree } from "@typescript-eslint/utils"; +import { getProp, getPropValue, hasProp } from "jsx-ast-utils"; + +// Mocking getProp, getPropValue, and hasProp +jest.mock("jsx-ast-utils", () => ({ + hasProp: jest.fn(), + getProp: jest.fn(), + getPropValue: jest.fn() +})); + +describe("hasNonEmptyProp", () => { + const attributes: TSESTree.JSXOpeningElement["attributes"] = [] as any; + const propName = "testProp"; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should return false if the property does not exist", () => { + (hasProp as jest.Mock).mockReturnValue(false); + expect(hasNonEmptyProp(attributes, propName)).toBe(false); + }); + + it("should return false if the property value is undefined", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue(undefined); + expect(hasNonEmptyProp(attributes, propName)).toBe(false); + }); + + it("should return false if the property value is null", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue(null); + expect(hasNonEmptyProp(attributes, propName)).toBe(false); + }); + + it("should return true if the property value is a non-empty string", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue("non-empty string"); + expect(hasNonEmptyProp(attributes, propName)).toBe(true); + }); + + it("should return false if the property value is an empty string", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue(" "); + expect(hasNonEmptyProp(attributes, propName)).toBe(false); + }); + + it("should return true if the property value is a non-zero number", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue(42); + expect(hasNonEmptyProp(attributes, propName)).toBe(true); + }); + + it("should return true if the property value is a boolean", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue(true); + expect(hasNonEmptyProp(attributes, propName)).toBe(true); + }); + + it("should return true if the property value is a non-empty array", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue([1, 2, 3]); + expect(hasNonEmptyProp(attributes, propName)).toBe(true); + }); + + it("should return false if the property value is an empty array", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue([]); + expect(hasNonEmptyProp(attributes, propName)).toBe(false); + }); + + it("should return true if the property value is a non-empty object", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue({ key: "value" }); + expect(hasNonEmptyProp(attributes, propName)).toBe(true); + }); + + it("should return false if the property value is an empty object", () => { + (hasProp as jest.Mock).mockReturnValue(true); + (getProp as jest.Mock).mockReturnValue({}); + (getPropValue as jest.Mock).mockReturnValue({}); + expect(hasNonEmptyProp(attributes, propName)).toBe(false); + }); +}); diff --git a/tests/lib/rules/utils/hasTextContentChild.test.ts b/tests/lib/rules/utils/hasTextContentChild.test.ts new file mode 100644 index 0000000..056963e --- /dev/null +++ b/tests/lib/rules/utils/hasTextContentChild.test.ts @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { hasTextContentChild } from "../../../../lib/util/hasTextContentChild"; +import { TSESTree } from "@typescript-eslint/types"; + +describe("hasTextContentChild", () => { + it("should return false when node is undefined", () => { + expect(hasTextContentChild(undefined)).toBe(false); + }); + + it("should return false when node.children is null or undefined", () => { + const node: TSESTree.JSXElement = { children: null } as any; + expect(hasTextContentChild(node)).toBe(false); + + const nodeUndefinedChildren: TSESTree.JSXElement = { children: undefined } as any; + expect(hasTextContentChild(nodeUndefinedChildren)).toBe(false); + }); + + it("should return false when node.children is an empty array", () => { + const node: TSESTree.JSXElement = { children: [] } as any; + expect(hasTextContentChild(node)).toBe(false); + }); + + it("should return false when node.children has no JSXText elements with non-whitespace content", () => { + const node: TSESTree.JSXElement = { + children: [{ type: "JSXElement" }, { type: "JSXExpressionContainer" }] + } as any; + expect(hasTextContentChild(node)).toBe(false); + }); + + it("should return true when node.children has at least one JSXText element with non-whitespace content", () => { + const node: TSESTree.JSXElement = { + children: [{ type: "JSXText", value: "Hello" }, { type: "JSXElement" }] + } as any; + expect(hasTextContentChild(node)).toBe(true); + }); + + it("should return false when node.children has only whitespace in JSXText elements", () => { + const node: TSESTree.JSXElement = { + children: [{ type: "JSXText", value: " " }] + } as any; + expect(hasTextContentChild(node)).toBe(false); + }); +}); diff --git a/tests/lib/rules/utils/hasTooltipParent.test.ts b/tests/lib/rules/utils/hasTooltipParent.test.ts index b048b33..5c356a5 100644 --- a/tests/lib/rules/utils/hasTooltipParent.test.ts +++ b/tests/lib/rules/utils/hasTooltipParent.test.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + import { hasToolTipParent } from "../../../../lib/util/hasTooltipParent"; import { TSESLint } from "@typescript-eslint/utils"; diff --git a/tests/lib/rules/utils/labelUtils.test.ts b/tests/lib/rules/utils/labelUtils.test.ts index 1d600e2..41506a4 100644 --- a/tests/lib/rules/utils/labelUtils.test.ts +++ b/tests/lib/rules/utils/labelUtils.test.ts @@ -1,54 +1,199 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { isInsideLabelTag } from "../../../../lib/util/labelUtils"; - -describe("isInsideLabelTag", function () { - it("should return true when nested within a Label tag", function () { - const context = { - getAncestors: () => [ - { - type: "JSXElement", - openingElement: { name: { name: "Label" } } - } - // Other ancestors as needed... - ] - }; - - const result = isInsideLabelTag(context); - - expect(result).toBe(true); - }); +import { hasAssociatedLabelViaAriaLabelledBy, hasAssociatedLabelViaAriaDescribedby } from "../../../../lib/util/labelUtils"; + +import { TSESTree, TSESLint, AST_NODE_TYPES } from "@typescript-eslint/utils"; // Use TSESTree types consistently + +describe("labelUtils", () => { + // Mock context with getSourceCode method + const mockContext = (): TSESLint.RuleContext => { + return { + getSourceCode: () => ({ + getText: () => "mocked text" + }) + } as unknown as TSESLint.RuleContext; + }; + // Define the test suite + describe("hasAssociatedLabelViaAriaLabelledBy", () => { + let context: TSESLint.RuleContext; + let openingElement: TSESTree.JSXOpeningElement; + + beforeEach(() => { + context = mockContext(); + openingElement = { + attributes: [] + } as unknown as TSESTree.JSXOpeningElement; + }); + + function createJSXAttribute(name: string, value: string | number | null): TSESTree.JSXAttribute { + return { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name } as TSESTree.JSXIdentifier, + value: value !== null ? ({ type: AST_NODE_TYPES.Literal, value } as TSESTree.Literal) : null, + loc: {} as TSESTree.SourceLocation, + range: [0, 0] + }; + } + + test("returns false if aria-labelledby is missing", () => { + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, context); + expect(result).toBe(false); + }); + + test("returns false if aria-labelledby is empty", () => { + openingElement.attributes = [createJSXAttribute("aria-labelledby", "")]; + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, context); + expect(result).toBe(false); + }); + + test("returns false if aria-labelledby value is not a string", () => { + openingElement.attributes = [createJSXAttribute("aria-labelledby", 123)]; + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, context); + expect(result).toBe(false); + }); + + test("returns false if referenced element by id does not exist", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "", + text: () => "" + }) + } as unknown as TSESLint.RuleContext; + + openingElement.attributes = [createJSXAttribute("aria-labelledby", "non-existing-id")]; + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, customContext); + expect(result).toBe(false); + }); + + test("returns true if aria-labelledby references an existing label element", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "", + text: () => "" + }) + } as unknown as TSESLint.RuleContext; + + openingElement.attributes = [createJSXAttribute("aria-labelledby", "existing-label-id")]; + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, customContext); + expect(result).toBe(true); + }); - it("should return true when nested within a label tag (case-insensitive)", function () { - const context = { - getAncestors: () => [ - { - type: "JSXElement", - openingElement: { name: { name: "label" } } - } - // Other ancestors as needed... - ] - }; + test("returns true if aria-labelledby references an existing non-label element", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "
Test Label
", + text: () => "
Test Label
" + }) + } as unknown as TSESLint.RuleContext; - const result = isInsideLabelTag(context); + openingElement.attributes = [createJSXAttribute("aria-labelledby", "existing-non-label-id")]; + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, customContext); + expect(result).toBe(true); + }); - expect(result).toBe(true); + test("returns true if aria-labelledby references both label and non-label elements", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "

Test Label

", + text: () => "

Test Label

" + }) + } as unknown as TSESLint.RuleContext; + + openingElement.attributes = [createJSXAttribute("aria-labelledby", "existing-label-id")]; + const result = hasAssociatedLabelViaAriaLabelledBy(openingElement, customContext); + expect(result).toBe(true); + }); }); - it("should return false when not nested within a Label tag", function () { - const context = { - getAncestors: () => [ - { - type: "JSXElement", - openingElement: { name: { name: "div" } } // Non-Label element - } - // Other ancestors as needed... - ] - }; + describe("hasAssociatedLabelViaAriaDescribedby", () => { + let context: TSESLint.RuleContext; + let openingElement: TSESTree.JSXOpeningElement; + + beforeEach(() => { + context = mockContext(); + openingElement = { + attributes: [] + } as unknown as TSESTree.JSXOpeningElement; + }); + + function createJSXAttribute(name: string, value: string | number | null): TSESTree.JSXAttribute { + return { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name } as TSESTree.JSXIdentifier, + value: value !== null ? ({ type: AST_NODE_TYPES.Literal, value } as TSESTree.Literal) : null, + loc: {} as TSESTree.SourceLocation, + range: [0, 0] + }; + } + + test("returns false if aria-describedby is missing", () => { + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, context); + expect(result).toBe(false); + }); + + test("returns false if aria-describedby is empty", () => { + openingElement.attributes = [createJSXAttribute("aria-describedby", "")]; + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, context); + expect(result).toBe(false); + }); + + test("returns false if aria-describedby value is not a string", () => { + openingElement.attributes = [createJSXAttribute("aria-describedby", 123)]; + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, context); + expect(result).toBe(false); + }); + + test("returns false if referenced element by id does not exist", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "", + text: () => "" + }) + } as unknown as TSESLint.RuleContext; + + openingElement.attributes = [createJSXAttribute("aria-describedby", "non-existing-id")]; + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, customContext); + expect(result).toBe(false); + }); + + test("returns true if aria-describedby references an existing label element", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "", + text: () => "" + }) + } as unknown as TSESLint.RuleContext; + + openingElement.attributes = [createJSXAttribute("aria-describedby", "existing-label-id")]; + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, customContext); + expect(result).toBe(true); + }); + + test("returns true if aria-describedby references an existing non-label element", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "
Test Label
", + text: () => "
Test Label
" + }) + } as unknown as TSESLint.RuleContext; + + openingElement.attributes = [createJSXAttribute("aria-describedby", "existing-non-label-id")]; + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, customContext); + expect(result).toBe(true); + }); - const result = isInsideLabelTag(context); + test("returns true if aria-describedby references both label and non-label elements", () => { + const customContext: TSESLint.RuleContext = { + getSourceCode: () => ({ + getText: () => "

Test Label

", + text: () => "

Test Label

" + }) + } as unknown as TSESLint.RuleContext; - expect(result).toBe(false); + openingElement.attributes = [createJSXAttribute("aria-describedby", "existing-label-id")]; + const result = hasAssociatedLabelViaAriaDescribedby(openingElement, customContext); + expect(result).toBe(true); + }); }); }); diff --git a/tests/lib/rules/visual-label-better-than-aria-suggestion.js b/tests/lib/rules/visual-label-better-than-aria-suggestion.test.ts similarity index 73% rename from tests/lib/rules/visual-label-better-than-aria-suggestion.js rename to tests/lib/rules/visual-label-better-than-aria-suggestion.test.ts index 8262ffc..92650c0 100644 --- a/tests/lib/rules/visual-label-better-than-aria-suggestion.js +++ b/tests/lib/rules/visual-label-better-than-aria-suggestion.test.ts @@ -1,31 +1,20 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -"use strict"; +import { Rule } from "eslint"; +import ruleTester from "./helper/ruleTester"; +import rule from "../../../lib/rules/visual-label-better-than-aria-suggestion"; -const { applicableComponents } = require("../../../lib/applicableComponents/buttonBasedComponents"); +import { applicableComponents } from "../../../lib/applicableComponents/buttonBasedComponents"; //------------------------------------------------------------------------------ // Requirements //------------------------------------------------------------------------------ -const RuleTester = require("eslint").RuleTester; - -const rule = require("../../../lib/rules/visual-label-better-than-aria-suggestion"); - -RuleTester.setDefaultConfig({ - parserOptions: { - ecmaVersion: 6, - ecmaFeatures: { - jsx: true - } - } -}); - //------------------------------------------------------------------------------ // Helper function to generate test cases //------------------------------------------------------------------------------ -function generateTestCases(componentName) { +const generateTestCases = (componentName: string) => { return { valid: [ `<><${componentName} id="some-id"/>`, @@ -40,7 +29,7 @@ function generateTestCases(componentName) { } ] }; -} +}; // Collect all test cases for all applicable components const allTestCases = applicableComponents.flatMap(component => generateTestCases(component)); @@ -49,8 +38,7 @@ const allTestCases = applicableComponents.flatMap(component => generateTestCases // Tests //------------------------------------------------------------------------------ -const ruleTester = new RuleTester(); -ruleTester.run("visual-label-better-than-aria-suggestion", rule, { +ruleTester.run("visual-label-better-than-aria-suggestion", rule as unknown as Rule.RuleModule, { valid: allTestCases.flatMap(test => test.valid), invalid: allTestCases.flatMap(test => test.invalid) }); diff --git a/tsconfig.json b/tsconfig.json index 6613001..8bde47e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -5,8 +5,7 @@ "allowUnusedLabels": false, "declaration": true, "declarationMap": false, - "module": "Node16", - "moduleResolution": "Node16", + "module": "CommonJS", "noImplicitReturns": true, "pretty": true, "resolveJsonModule": true, From 1de6006c7239732a7a00a1f1dd09170654c89af8 Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Wed, 6 Nov 2024 17:55:07 +0000 Subject: [PATCH 03/10] tests --- .../rules/utils/hasLabelledChilImage.test.ts | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 tests/lib/rules/utils/hasLabelledChilImage.test.ts diff --git a/tests/lib/rules/utils/hasLabelledChilImage.test.ts b/tests/lib/rules/utils/hasLabelledChilImage.test.ts new file mode 100644 index 0000000..1c514e3 --- /dev/null +++ b/tests/lib/rules/utils/hasLabelledChilImage.test.ts @@ -0,0 +1,156 @@ +// Import necessary dependencies and mock functions +import { + hasLabelledChildImage, + isImageHidden, + hasAccessibilityAttributes, + isJSXIdentifierWithName +} from "../../../../lib/util/hasLabelledChildImage"; +import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/types"; +import { fluentImageComponents, imageDomNodes } from "../../../../lib/applicableComponents/imageBasedComponents"; +const mergedImageComponents = [...fluentImageComponents, ...imageDomNodes]; + +// Helper function to create mock loc and range +const createMockLocRange = () => ({ + loc: { start: { line: 0, column: 0 }, end: { line: 0, column: 0 } }, + range: [0, 0] as [number, number] +}); + +// Unit tests +describe("isJSXIdentifierWithName", () => { + it("returns true for a JSXIdentifier with a valid name", () => { + const name: TSESTree.JSXIdentifier = { type: AST_NODE_TYPES.JSXIdentifier, name: "img", ...createMockLocRange() }; + expect(isJSXIdentifierWithName(name, mergedImageComponents)).toBe(true); + }); + + it("returns false for a JSXIdentifier with an invalid name", () => { + const name: TSESTree.JSXIdentifier = { type: AST_NODE_TYPES.JSXIdentifier, name: "div", ...createMockLocRange() }; + expect(isJSXIdentifierWithName(name, mergedImageComponents)).toBe(false); + }); +}); + +describe("hasAccessibilityAttributes", () => { + it("returns true if any accessible attribute is non-empty", () => { + const attributes: TSESTree.JSXOpeningElement["attributes"] = [ + { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "alt", ...createMockLocRange() }, + value: { + type: AST_NODE_TYPES.Literal, + value: "An image description", + raw: '"An image description"', + ...createMockLocRange() + }, + ...createMockLocRange() + } + ]; + expect(hasAccessibilityAttributes(attributes)).toBe(true); + }); + + it("returns false if no accessible attribute is present", () => { + const attributes: TSESTree.JSXOpeningElement["attributes"] = [ + { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "aria-hidden", ...createMockLocRange() }, + value: { type: AST_NODE_TYPES.Literal, value: "true", raw: '"true"', ...createMockLocRange() }, + ...createMockLocRange() + } + ]; + expect(hasAccessibilityAttributes(attributes)).toBe(false); + }); +}); + +describe("isImageHidden", () => { + it("returns true if `aria-hidden` is set", () => { + const attributes: TSESTree.JSXOpeningElement["attributes"] = [ + { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "aria-hidden", ...createMockLocRange() }, + value: { type: AST_NODE_TYPES.Literal, value: "true", raw: '"true"', ...createMockLocRange() }, + ...createMockLocRange() + } + ]; + expect(isImageHidden(attributes)).toBe(true); + }); + + it("returns true if `alt` attribute is empty", () => { + const attributes: TSESTree.JSXOpeningElement["attributes"] = [ + { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "alt", ...createMockLocRange() }, + value: { type: AST_NODE_TYPES.Literal, value: "", raw: '""', ...createMockLocRange() }, + ...createMockLocRange() + } + ]; + expect(isImageHidden(attributes)).toBe(true); + }); + + it("returns false if `alt` attribute is non-empty", () => { + const attributes: TSESTree.JSXOpeningElement["attributes"] = [ + { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "alt", ...createMockLocRange() }, + value: { type: AST_NODE_TYPES.Literal, value: "Image description", raw: '"Image description"', ...createMockLocRange() }, + ...createMockLocRange() + } + ]; + expect(isImageHidden(attributes)).toBe(false); + }); +}); + +describe("hasLabelledChildImage", () => { + it("returns true if a child image component with accessibility attributes is found", () => { + const mockChild: TSESTree.JSXElement = { + type: AST_NODE_TYPES.JSXElement, + openingElement: { + type: AST_NODE_TYPES.JSXOpeningElement, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "img", ...createMockLocRange() }, + attributes: [ + { + type: AST_NODE_TYPES.JSXAttribute, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "alt", ...createMockLocRange() }, + value: { type: AST_NODE_TYPES.Literal, value: "description", raw: '"description"', ...createMockLocRange() }, + ...createMockLocRange() + } + ], + selfClosing: false, + ...createMockLocRange() + }, + closingElement: null, + children: [], + ...createMockLocRange() + }; + + const node: TSESTree.JSXElement = { + type: AST_NODE_TYPES.JSXElement, + openingElement: { + type: AST_NODE_TYPES.JSXOpeningElement, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "Container", ...createMockLocRange() }, + attributes: [], + selfClosing: false, + ...createMockLocRange() + }, + closingElement: null, + children: [mockChild], + ...createMockLocRange() + }; + expect(hasLabelledChildImage(node)).toBe(true); + }); + + it("returns false if no image component is found", () => { + const node: TSESTree.JSXElement = { + type: AST_NODE_TYPES.JSXElement, + openingElement: { + type: AST_NODE_TYPES.JSXOpeningElement, + name: { type: AST_NODE_TYPES.JSXIdentifier, name: "div", ...createMockLocRange() }, + attributes: [], + selfClosing: false, + ...createMockLocRange() + }, + closingElement: null, + children: [], + ...createMockLocRange() + }; + + expect(hasLabelledChildImage(node)).toBe(false); + }); +}); From 3ad2214d637d18f268ee6034798207f02b780bcb Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Thu, 7 Nov 2024 17:57:39 +0000 Subject: [PATCH 04/10] fixed broken test --- .../linkBasedComponents.js | 4 +- ...nput-components-require-accessible-name.ts | 4 +- ...isual-label-better-than-aria-suggestion.ts | 2 +- lib/util/hasLabelledChildImage.ts | 18 ++++- ...components-require-accessible-name.test.ts | 3 +- .../lib/rules/link-missing-labelling.test.ts | 79 ++++++++++--------- 6 files changed, 65 insertions(+), 45 deletions(-) diff --git a/lib/applicableComponents/linkBasedComponents.js b/lib/applicableComponents/linkBasedComponents.js index 780ca2e..2c92b02 100644 --- a/lib/applicableComponents/linkBasedComponents.js +++ b/lib/applicableComponents/linkBasedComponents.js @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -const linkBasedComponents = ["Link", "a"]; // , "BreadcrumbButton" - +// const linkBasedComponents = ["Link", "a"]; // , "BreadcrumbButton" +const linkBasedComponents = ["Link"]; module.exports = { linkBasedComponents }; diff --git a/lib/rules/input-components-require-accessible-name.ts b/lib/rules/input-components-require-accessible-name.ts index 9fa0c88..0a214d0 100644 --- a/lib/rules/input-components-require-accessible-name.ts +++ b/lib/rules/input-components-require-accessible-name.ts @@ -7,6 +7,7 @@ import { isInsideLabelTag, hasAssociatedLabelViaHtmlFor, hasAssociatedLabelViaAr import { hasFieldParent } from "../util/hasFieldParent"; import { applicableComponents } from "../applicableComponents/inputBasedComponents"; import { JSXOpeningElement } from "estree-jsx"; +import { hasNonEmptyProp } from "../util/hasNonEmptyProp"; //------------------------------------------------------------------------------ // Rule Definition @@ -17,7 +18,7 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ meta: { // possible error messages for the rule messages: { - missingLabelOnInput: `Accessibility - input fields must have a aria label associated with it: ${applicableComponents.join( + missingLabelOnInput: `Accessibility - input fields must have an accessible label associated with it: ${applicableComponents.join( ", " )}` }, @@ -43,6 +44,7 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ // wrapped in Label tag, labelled with htmlFor, labelled with aria-labelledby if ( + hasNonEmptyProp(node.attributes, "aria-label") || hasFieldParent(context) || isInsideLabelTag(context) || hasAssociatedLabelViaHtmlFor(node, context) || diff --git a/lib/rules/visual-label-better-than-aria-suggestion.ts b/lib/rules/visual-label-better-than-aria-suggestion.ts index 4f3e000..31c45f6 100644 --- a/lib/rules/visual-label-better-than-aria-suggestion.ts +++ b/lib/rules/visual-label-better-than-aria-suggestion.ts @@ -20,7 +20,7 @@ const rule = ESLintUtils.RuleCreator.withoutDocs({ }, type: "suggestion", // `problem`, `suggestion`, or `layout` docs: { - description: "Visual label is better than an aria-label", + description: "Visual label is better than an aria-label because sighted users can't read the aria-label text.", recommended: "strict", url: undefined // URL to the documentation page for this rule }, diff --git a/lib/util/hasLabelledChildImage.ts b/lib/util/hasLabelledChildImage.ts index a5f71d5..443941c 100644 --- a/lib/util/hasLabelledChildImage.ts +++ b/lib/util/hasLabelledChildImage.ts @@ -25,6 +25,7 @@ const isJSXIdentifierWithName = (name: TSESTree.JSXTagNameExpression, validNames * @returns boolean */ const hasLabelledChildImage = (node: TSESTree.JSXElement): boolean => { + console.log("node::", node); if (!node.children || node.children.length === 0) { return false; } @@ -32,6 +33,10 @@ const hasLabelledChildImage = (node: TSESTree.JSXElement): boolean => { return flattenChildren(node).some(child => { if (child.type === "JSXElement" && isJSXIdentifierWithName(child.openingElement.name, mergedImageComponents)) { const attributes = child.openingElement.attributes; + console.log("attributes::", attributes); + console.log("hasAccessibilityAttributes(attributes)", hasAccessibilityAttributes(attributes)); + console.log("!isImageHidden(attributes)", !isImageHidden(attributes)); + return !isImageHidden(attributes) && hasAccessibilityAttributes(attributes); } return false; @@ -58,17 +63,28 @@ const hasAccessibilityAttributes = (attributes: TSESTree.JSXOpeningElement["attr * @returns boolean */ const isImageHidden = (attributes: TSESTree.JSXOpeningElement["attributes"]): boolean => { + // Check if the image has the `aria-hidden` attribute if (hasProp(attributes as unknown as JSXOpeningElement["attributes"], "aria-hidden")) { return true; } + // Check if the image has an `aria-label` attribute with a non-empty value + const ariaLabelProp = getProp(attributes as unknown as JSXOpeningElement["attributes"], "aria-label"); + if (ariaLabelProp) { + const ariaLabelValue = getPropValue(ariaLabelProp); + if (ariaLabelValue) { + return false; // If `aria-label` is present and has a value, the image is not hidden + } + } + + // Check if the image has an `alt` attribute and return true if the `alt` value is falsy const altProp = getProp(attributes as unknown as JSXOpeningElement["attributes"], "alt"); if (altProp) { const altValue = getPropValue(altProp); return !altValue; // Returns true if `altValue` is falsy (e.g., empty string, null, or undefined) } - return true; // If `alt` is not present, consider the image hidden + return true; // If neither `alt` nor `aria-label` is present, consider the image hidden }; export { hasLabelledChildImage, isImageHidden, hasAccessibilityAttributes, isJSXIdentifierWithName }; diff --git a/tests/lib/rules/input-components-require-accessible-name.test.ts b/tests/lib/rules/input-components-require-accessible-name.test.ts index f29bacb..51109c2 100644 --- a/tests/lib/rules/input-components-require-accessible-name.test.ts +++ b/tests/lib/rules/input-components-require-accessible-name.test.ts @@ -21,7 +21,8 @@ function generateTestCases(componentName: string) { ``, ``, ``, - `<${componentName} />` + `<${componentName} />`, + `<${componentName} aria-label="this is my component" />` ], invalid: [ { diff --git a/tests/lib/rules/link-missing-labelling.test.ts b/tests/lib/rules/link-missing-labelling.test.ts index f094522..8f169ec 100644 --- a/tests/lib/rules/link-missing-labelling.test.ts +++ b/tests/lib/rules/link-missing-labelling.test.ts @@ -14,43 +14,44 @@ function generateTestCases(componentName: string, imageName: string) { return { valid: [ // Valid cases - `<${componentName} href="https://www.bing.com">This is a link`, - `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" />`, - `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, - `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, - `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" aria-label="The girl with the dog." />`, - `<${componentName} href="https://www.bing.com" aria-label="The girl with the dog."><${imageName} src="img_girl.jpg" />`, - `<${componentName} href="https://www.bing.com" title="The girl with the dog."><${imageName} src="img_girl.jpg" />`, - `<><${componentName} href="https://www.bing.com" aria-labelledby="my-label-2"><${imageName} src="img_girl.jpg" />`, - `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" alt="The girl with the dog." />` + // `<${componentName} href="https://www.bing.com">This is a link`, + // `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" />`, + // `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, + // `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, + `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" aria-label="The girl with the dog." />` + // `<${componentName} href="https://www.bing.com" aria-label="The girl with the dog."><${imageName} src="img_girl.jpg" />` + // `<${componentName} href="https://www.bing.com" title="The girl with the dog."><${imageName} src="img_girl.jpg" />`, + // `<><${componentName} href="https://www.bing.com" aria-labelledby="my-label-2"><${imageName} src="img_girl.jpg" />`, + // `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" alt="The girl with the dog." />` ], - invalid: [ - // Invalid cases - { - code: `<${componentName} />`, - errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] - }, - { - code: `<${componentName}><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, - errors: [{ messageId: "missingHref" }] - }, - { - code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" />`, - errors: [{ messageId: "missingAriaLabel" }] - }, - { - code: `<${componentName}><${imageName} src="img_girl.jpg" />`, - errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] - }, - { - code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, - errors: [{ messageId: "missingAriaLabel" }] - }, - { - code: `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" />`, - errors: [{ messageId: "missingAriaLabel" }] - } - ] + invalid: [] + // invalid: [ + // // Invalid cases + // { + // code: `<${componentName} />`, + // errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] + // }, + // { + // code: `<${componentName}><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, + // errors: [{ messageId: "missingHref" }] + // }, + // { + // code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" />`, + // errors: [{ messageId: "missingAriaLabel" }] + // }, + // { + // code: `<${componentName}><${imageName} src="img_girl.jpg" />`, + // errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] + // }, + // { + // code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, + // errors: [{ messageId: "missingAriaLabel" }] + // }, + // { + // code: `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" />`, + // errors: [{ messageId: "missingAriaLabel" }] + // } + // ] }; } @@ -68,9 +69,9 @@ function generateAllTestCases() { }); // Also generate test cases for each native DOM image node (e.g., img, svg) - imageDomNodes.forEach(imageComponent => { - testSets.push(generateTestCases(linkComponent, imageComponent)); - }); + // imageDomNodes.forEach(imageComponent => { + // testSets.push(generateTestCases(linkComponent, imageComponent)); + // }); }); return testSets; From 1eb5d03701a2b0f4e13358a4f936d54217466962 Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Thu, 7 Nov 2024 18:01:13 +0000 Subject: [PATCH 05/10] fixed broken test --- .../linkBasedComponents.js | 4 +- .../lib/rules/link-missing-labelling.test.ts | 80 +++++++++---------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/lib/applicableComponents/linkBasedComponents.js b/lib/applicableComponents/linkBasedComponents.js index 2c92b02..780ca2e 100644 --- a/lib/applicableComponents/linkBasedComponents.js +++ b/lib/applicableComponents/linkBasedComponents.js @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// const linkBasedComponents = ["Link", "a"]; // , "BreadcrumbButton" -const linkBasedComponents = ["Link"]; +const linkBasedComponents = ["Link", "a"]; // , "BreadcrumbButton" + module.exports = { linkBasedComponents }; diff --git a/tests/lib/rules/link-missing-labelling.test.ts b/tests/lib/rules/link-missing-labelling.test.ts index 8f169ec..1655dfc 100644 --- a/tests/lib/rules/link-missing-labelling.test.ts +++ b/tests/lib/rules/link-missing-labelling.test.ts @@ -14,44 +14,44 @@ function generateTestCases(componentName: string, imageName: string) { return { valid: [ // Valid cases - // `<${componentName} href="https://www.bing.com">This is a link`, - // `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" />`, - // `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, - // `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, - `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" aria-label="The girl with the dog." />` - // `<${componentName} href="https://www.bing.com" aria-label="The girl with the dog."><${imageName} src="img_girl.jpg" />` - // `<${componentName} href="https://www.bing.com" title="The girl with the dog."><${imageName} src="img_girl.jpg" />`, - // `<><${componentName} href="https://www.bing.com" aria-labelledby="my-label-2"><${imageName} src="img_girl.jpg" />`, - // `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" alt="The girl with the dog." />` + `<${componentName} href="https://www.bing.com">This is a link`, + `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" />`, + `<${componentName} href="https://www.bing.com">This is a link<${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, + `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, + `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" aria-label="The girl with the dog." />`, + `<${componentName} href="https://www.bing.com" aria-label="The girl with the dog."><${imageName} src="img_girl.jpg" />`, + `<${componentName} href="https://www.bing.com" title="The girl with the dog."><${imageName} src="img_girl.jpg" />`, + `<><${componentName} href="https://www.bing.com" aria-labelledby="my-label-2"><${imageName} src="img_girl.jpg" />`, + `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" alt="The girl with the dog." />` ], - invalid: [] - // invalid: [ - // // Invalid cases - // { - // code: `<${componentName} />`, - // errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] - // }, - // { - // code: `<${componentName}><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, - // errors: [{ messageId: "missingHref" }] - // }, - // { - // code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" />`, - // errors: [{ messageId: "missingAriaLabel" }] - // }, - // { - // code: `<${componentName}><${imageName} src="img_girl.jpg" />`, - // errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] - // }, - // { - // code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, - // errors: [{ messageId: "missingAriaLabel" }] - // }, - // { - // code: `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" />`, - // errors: [{ messageId: "missingAriaLabel" }] - // } - // ] + + invalid: [ + // Invalid cases + { + code: `<${componentName} />`, + errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] + }, + { + code: `<${componentName}><${imageName} src="img_girl.jpg" alt="The girl with the dog." />`, + errors: [{ messageId: "missingHref" }] + }, + { + code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" />`, + errors: [{ messageId: "missingAriaLabel" }] + }, + { + code: `<${componentName}><${imageName} src="img_girl.jpg" />`, + errors: [{ messageId: "missingHref" }, { messageId: "missingAriaLabel" }] + }, + { + code: `<${componentName} href="https://www.bing.com"><${imageName} src="img_girl.jpg" alt="" aria-hidden="true" />`, + errors: [{ messageId: "missingAriaLabel" }] + }, + { + code: `<${componentName} href="https://www.bing.com"><${imageName} src="img1.jpg" /><${imageName} src="img2.jpg" />`, + errors: [{ messageId: "missingAriaLabel" }] + } + ] }; } @@ -69,9 +69,9 @@ function generateAllTestCases() { }); // Also generate test cases for each native DOM image node (e.g., img, svg) - // imageDomNodes.forEach(imageComponent => { - // testSets.push(generateTestCases(linkComponent, imageComponent)); - // }); + imageDomNodes.forEach(imageComponent => { + testSets.push(generateTestCases(linkComponent, imageComponent)); + }); }); return testSets; From 6c0909433e22e5c0d14ffa58d6a2f92aa857e95e Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Thu, 7 Nov 2024 18:02:22 +0000 Subject: [PATCH 06/10] updated rules --- README.md | 2 +- docs/rules/visual-label-better-than-aria-suggestion.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 571fdfa..23c6a89 100644 --- a/README.md +++ b/README.md @@ -141,6 +141,6 @@ Any use of third-party trademarks or logos are subject to those third-party's po | [tablist-and-tabs-need-labelling](docs/rules/tablist-and-tabs-need-labelling.md) | This rule aims to ensure that Tabs with icons but no text labels have an accessible name and that Tablist is properly labeled. | βœ… | | | | [toolbar-missing-aria](docs/rules/toolbar-missing-aria.md) | Accessibility: Toolbars need accessible labelling: aria-label or aria-labelledby | βœ… | | | | [tooltip-not-recommended](docs/rules/tooltip-not-recommended.md) | Accessibility: Prefer text content or aria over a tooltip for these components MenuItem, SpinButton | βœ… | | | -| [visual-label-better-than-aria-suggestion](docs/rules/visual-label-better-than-aria-suggestion.md) | Visual label is better than an aria-label | | βœ… | | +| [visual-label-better-than-aria-suggestion](docs/rules/visual-label-better-than-aria-suggestion.md) | Visual label is better than an aria-label because sighted users can't read the aria-label text. | | βœ… | | diff --git a/docs/rules/visual-label-better-than-aria-suggestion.md b/docs/rules/visual-label-better-than-aria-suggestion.md index 7dff8c6..6cdd715 100644 --- a/docs/rules/visual-label-better-than-aria-suggestion.md +++ b/docs/rules/visual-label-better-than-aria-suggestion.md @@ -1,4 +1,4 @@ -# Visual label is better than an aria-label (`@microsoft/fluentui-jsx-a11y/visual-label-better-than-aria-suggestion`) +# Visual label is better than an aria-label because sighted users can't read the aria-label text (`@microsoft/fluentui-jsx-a11y/visual-label-better-than-aria-suggestion`) ⚠️ This rule _warns_ in the βœ… `recommended` config. From ce4749ee8336e015d8aa7e5e09484459eada5cae Mon Sep 17 00:00:00 2001 From: Aubrey Quinn Date: Fri, 8 Nov 2024 17:24:13 +0000 Subject: [PATCH 07/10] fixed --- .../labelBasedComponents.ts | 4 ++ ...isual-label-better-than-aria-suggestion.ts | 2 +- ...components-require-accessible-name.test.ts | 64 ++++++++++++++----- ...-label-better-than-aria-suggestion.test.ts | 2 +- 4 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 lib/applicableComponents/labelBasedComponents.ts diff --git a/lib/applicableComponents/labelBasedComponents.ts b/lib/applicableComponents/labelBasedComponents.ts new file mode 100644 index 0000000..4f847a8 --- /dev/null +++ b/lib/applicableComponents/labelBasedComponents.ts @@ -0,0 +1,4 @@ +const labelBasedComponents = ["Label", "label"]; +const elementsUsedAsLabels = ["div", "span", "p", "h1", "h2", "h3", "h4", "h5", "h6"]; + +export { labelBasedComponents, elementsUsedAsLabels }; diff --git a/lib/rules/visual-label-better-than-aria-suggestion.ts b/lib/rules/visual-label-better-than-aria-suggestion.ts index 31c45f6..c2226ec 100644 --- a/lib/rules/visual-label-better-than-aria-suggestion.ts +++ b/lib/rules/visual-label-better-than-aria-suggestion.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { hasNonEmptyProp } from "../util/hasNonEmptyProp"; -import { applicableComponents } from "../applicableComponents/buttonBasedComponents"; +import { applicableComponents } from "../applicableComponents/inputBasedComponents"; import { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; import { elementType } from "jsx-ast-utils"; import { JSXOpeningElement } from "estree-jsx"; diff --git a/tests/lib/rules/input-components-require-accessible-name.test.ts b/tests/lib/rules/input-components-require-accessible-name.test.ts index 51109c2..e180440 100644 --- a/tests/lib/rules/input-components-require-accessible-name.test.ts +++ b/tests/lib/rules/input-components-require-accessible-name.test.ts @@ -9,19 +9,34 @@ import { Rule } from "eslint"; import ruleTester from "./helper/ruleTester"; import rule from "../../../lib/rules/input-components-require-accessible-name"; import { applicableComponents } from "../../../lib/applicableComponents/inputBasedComponents"; +import { labelBasedComponents, elementsUsedAsLabels } from "../../../lib/applicableComponents/labelBasedComponents"; //------------------------------------------------------------------------------ // Helper function to generate test cases //------------------------------------------------------------------------------ -function generateTestCases(componentName: string) { +function generateTestCases(labelComponent: string, componentName: string) { return { valid: [ - `<><${componentName} id="some-id"/>`, - `<><${componentName} id="some-id" aria-labelledby="test-span"/>`, - ``, - ``, - ``, - `<${componentName} />`, + `<><${labelComponent} id="test-span">Some Label<${componentName} id="some-id" aria-labelledby="test-span"/>` + ], + invalid: [ + { + code: `<><${labelComponent} id="test-span-2">Some Label<${componentName} id="some-id" aria-labelledby="test-span"/>`, + errors: [{ messageId: "missingLabelOnInput" }] + } + ] + }; +} + +function generateTestCasesLabel(labelComponent: string, componentName: string) { + return { + valid: [ + `<><${labelComponent} htmlFor="some-id">Some Label<${componentName} id="some-id"/>`, + `<><${labelComponent} id="test-span">Some Label<${componentName} id="some-id" aria-labelledby="test-span"/>`, + `<${labelComponent}>test`, + `<${labelComponent}>test<${componentName} />`, + `<${labelComponent}>test<${componentName} />`, + `<${componentName} />`, `<${componentName} aria-label="this is my component" />` ], invalid: [ @@ -30,11 +45,11 @@ function generateTestCases(componentName: string) { errors: [{ messageId: "missingLabelOnInput" }] }, { - code: `<>