diff --git a/README.md b/README.md index 94aa29e..420bf64 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,7 @@ Any use of third-party trademarks or logos are subject to those third-party's po | [no-empty-buttons](docs/rules/no-empty-buttons.md) | Accessibility: Button, ToggleButton, SplitButton, MenuButton, CompoundButton must either text content or icon or child component | ✅ | | | | [no-empty-components](docs/rules/no-empty-components.md) | FluentUI components should not be empty | ✅ | | | | [prefer-aria-over-title-attribute](docs/rules/prefer-aria-over-title-attribute.md) | The title attribute is not consistently read by screen readers, and its behavior can vary depending on the screen reader and the user's settings. | | ✅ | 🔧 | +| [prefer-disabledfocusable-over-disabled](docs/rules/prefer-disabledfocusable-over-disabled.md) | Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility | | ✅ | 🔧 | | [progressbar-needs-labelling](docs/rules/progressbar-needs-labelling.md) | Accessibility: Progressbar must have aria-valuemin, aria-valuemax, aria-valuenow, aria-describedby and either aria-label or aria-labelledby attributes | ✅ | | | | [radio-button-missing-label](docs/rules/radio-button-missing-label.md) | Accessibility: Radio button without label must have an accessible and visual label: aria-labelledby | ✅ | | | | [radiogroup-missing-label](docs/rules/radiogroup-missing-label.md) | Accessibility: RadioGroup without label must have an accessible and visual label: aria-labelledby | ✅ | | | diff --git a/docs/rules/prefer-disabledfocusable-over-disabled.md b/docs/rules/prefer-disabledfocusable-over-disabled.md new file mode 100644 index 0000000..2b0cb3f --- /dev/null +++ b/docs/rules/prefer-disabledfocusable-over-disabled.md @@ -0,0 +1,139 @@ +# Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility (`@microsoft/fluentui-jsx-a11y/prefer-disabledfocusable-over-disabled`) + +⚠️ This rule _warns_ in the ✅ `recommended` config. + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +When components are in a loading state, prefer using `disabledFocusable` over `disabled` to maintain proper keyboard navigation flow and accessibility. + +## Rule Details + +This rule encourages the use of `disabledFocusable` instead of `disabled` when components have loading state indicators. This ensures: + +1. **Keyboard Navigation**: The component remains in the keyboard tab order, allowing users to discover and navigate to it +2. **Screen Reader Compatibility**: Screen reader users can still navigate to and understand the component's state +3. **Loading State Awareness**: Users understand that the component is temporarily unavailable due to loading, not permanently disabled +4. **Consistent UX**: Provides a more predictable and accessible user experience + +### Accessibility Impact + +- `disabled` removes elements completely from the tab order (tabindex="-1") +- `disabledFocusable` keeps elements in the tab order while conveying disabled state via `aria-disabled="true"` +- Loading states are temporary conditions where users benefit from knowing the component exists and will become available + +### Applicable Components + +This rule applies to FluentUI components that support both `disabled` and `disabledFocusable` props: + +**Button Components:** `Button`, `ToggleButton`, `CompoundButton`, `MenuButton`, `SplitButton` +**Form Controls:** `Checkbox`, `Radio`, `Switch` +**Input Components:** `Input`, `Textarea`, `Combobox`, `Dropdown`, `SpinButton`, `Slider`, `DatePicker`, `TimePicker` +**Other Interactive:** `Link`, `Tab` + +### Loading State Indicators + +The rule detects these loading-related props: +- `loading` +- `isLoading` +- `pending` +- `isPending` +- `busy` +- `isBusy` + +## Examples + +### ❌ Incorrect + +```jsx + + + + + + +``` + +### ✅ Correct + +```jsx + + + + + + + + + + + + + + + + +``` + +## Edge Cases & Considerations + +### Both Props Present +If both `disabled` and `disabledFocusable` are present, this rule will not trigger as it represents a different configuration issue. + +```jsx + + +``` + +### Non-Loading Disabled States +The rule only applies when both disabled AND loading states are present: + +```jsx + + +``` + +### Complex Expressions +The rule works with boolean expressions and variables: + +```jsx + + + + + +``` + +## When Not To Use It + +You may want to disable this rule if: + +1. **Intentional UX Decision**: You specifically want loading components removed from tab order +2. **Legacy Codebase**: Existing implementations rely on specific disabled behavior during loading +3. **Custom Loading Patterns**: Your application uses non-standard loading state management + +However, disabling this rule is generally **not recommended** as it reduces accessibility. + +## Automatic Fixes + +The rule provides automatic fixes that replace `disabled` with `disabledFocusable` while preserving the original prop value: + +```jsx +// Before fix + + +// After fix + +``` + +## Related Rules + +- [`no-empty-buttons`](./no-empty-buttons.md) - Ensures buttons have content or accessible labeling +- [`prefer-aria-over-title-attribute`](./prefer-aria-over-title-attribute.md) - Improves screen reader compatibility + +## Further Reading + +- [WAI-ARIA: Keyboard Interface](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/) +- [FluentUI Accessibility Guidelines](https://react.fluentui.dev/?path=/docs/concepts-developer-accessibility--page) +- [Understanding ARIA: disabled vs aria-disabled](https://css-tricks.com/making-disabled-buttons-more-inclusive/) diff --git a/lib/applicableComponents/disabledFocusableComponents.ts b/lib/applicableComponents/disabledFocusableComponents.ts new file mode 100644 index 0000000..f4b6ae3 --- /dev/null +++ b/lib/applicableComponents/disabledFocusableComponents.ts @@ -0,0 +1,36 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * FluentUI components that support both 'disabled' and 'disabledFocusable' props + * These are components where the rule should apply + */ +const disabledFocusableComponents = [ + // Button components + "Button", + "ToggleButton", + "CompoundButton", + "MenuButton", + "SplitButton", + + // Form controls + "Checkbox", + "Radio", + "Switch", + + // Input components + "Input", + "Textarea", + "Combobox", + "Dropdown", + "SpinButton", + "Slider", + "DatePicker", + "TimePicker", + + // Other interactive components + "Link", + "Tab" +] as const; + +export { disabledFocusableComponents }; diff --git a/lib/index.ts b/lib/index.ts index 8fd98c1..1c063e5 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -41,6 +41,7 @@ module.exports = { "@microsoft/fluentui-jsx-a11y/no-empty-buttons": "error", "@microsoft/fluentui-jsx-a11y/no-empty-components": "error", "@microsoft/fluentui-jsx-a11y/prefer-aria-over-title-attribute": "warn", + "@microsoft/fluentui-jsx-a11y/prefer-disabledfocusable-over-disabled": "warn", "@microsoft/fluentui-jsx-a11y/progressbar-needs-labelling": "error", "@microsoft/fluentui-jsx-a11y/radio-button-missing-label": "error", "@microsoft/fluentui-jsx-a11y/radiogroup-missing-label": "error", @@ -83,6 +84,7 @@ module.exports = { "no-empty-buttons": rules.noEmptyButtons, "no-empty-components": rules.noEmptyComponents, "prefer-aria-over-title-attribute": rules.preferAriaOverTitleAttribute, + "prefer-disabledfocusable-over-disabled": rules.preferDisabledFocusableOverDisabled, "progressbar-needs-labelling": rules.progressbarNeedsLabelling, "radio-button-missing-label": rules.radioButtonMissingLabel, "radiogroup-missing-label": rules.radiogroupMissingLabel, diff --git a/lib/rules/index.ts b/lib/rules/index.ts index d0db540..c31ac56 100644 --- a/lib/rules/index.ts +++ b/lib/rules/index.ts @@ -39,3 +39,4 @@ export { default as tablistAndTabsNeedLabelling } from "./tablist-and-tabs-need- export { default as toolbarMissingAria } from "./toolbar-missing-aria"; export { default as tooltipNotRecommended } from "./tooltip-not-recommended"; export { default as visualLabelBetterThanAriaSuggestion } from "./visual-label-better-than-aria-suggestion"; +export { default as preferDisabledFocusableOverDisabled } from "./prefer-disabledfocusable-over-disabled"; diff --git a/lib/rules/prefer-disabledfocusable-over-disabled.ts b/lib/rules/prefer-disabledfocusable-over-disabled.ts new file mode 100644 index 0000000..88c9232 --- /dev/null +++ b/lib/rules/prefer-disabledfocusable-over-disabled.ts @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; +import { elementType } from "jsx-ast-utils"; +import { hasNonEmptyProp } from "../util/hasNonEmptyProp"; +import { hasLoadingState, getLoadingStateProp } from "../util/hasLoadingState"; +import { disabledFocusableComponents } from "../applicableComponents/disabledFocusableComponents"; +import { JSXOpeningElement } from "estree-jsx"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const rule = ESLintUtils.RuleCreator.withoutDocs({ + defaultOptions: [], + meta: { + messages: { + preferDisabledFocusable: + "Accessibility: Prefer 'disabledFocusable={{{{loadingProp}}}}}' over 'disabled={{{{loadingProp}}}}}' when component has loading state '{{loadingProp}}' to maintain keyboard navigation accessibility", + preferDisabledFocusableGeneric: + "Accessibility: Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility" + }, + type: "suggestion", // This is a suggestion for better accessibility + docs: { + description: + "Prefer 'disabledFocusable' over 'disabled' when component has loading state to maintain keyboard navigation accessibility", + recommended: "warn", + url: "https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/" + }, + fixable: "code", // Allow auto-fixing + schema: [] + }, + + create(context) { + return { + JSXOpeningElement(node: TSESTree.JSXOpeningElement) { + const componentName = elementType(node as JSXOpeningElement); + + // Check if this is an applicable component + if (!disabledFocusableComponents.includes(componentName as any)) { + return; + } + + // Check if component has 'disabled' prop + const hasDisabled = hasNonEmptyProp(node.attributes, "disabled"); + if (!hasDisabled) { + return; + } + + // Check if component has loading state + const hasLoading = hasLoadingState(node.attributes); + if (!hasLoading) { + return; + } + + // Check if component already has disabledFocusable (avoid conflicts) + const hasDisabledFocusable = hasNonEmptyProp(node.attributes, "disabledFocusable"); + if (hasDisabledFocusable) { + return; // Don't report if both are present - that's a different issue + } + + const loadingProp = getLoadingStateProp(node.attributes); + + context.report({ + node, + messageId: loadingProp ? "preferDisabledFocusable" : "preferDisabledFocusableGeneric", + data: { + loadingProp: loadingProp || "loading" + }, + fix(fixer) { + // Find the disabled attribute and replace it with disabledFocusable + const disabledAttr = node.attributes.find( + attr => attr.type === "JSXAttribute" && attr.name?.type === "JSXIdentifier" && attr.name.name === "disabled" + ); + + if (disabledAttr && disabledAttr.type === "JSXAttribute" && disabledAttr.name) { + return fixer.replaceText(disabledAttr.name, "disabledFocusable"); + } + return null; + } + }); + } + }; + } +}); + +export default rule; diff --git a/lib/util/hasLoadingState.ts b/lib/util/hasLoadingState.ts new file mode 100644 index 0000000..8824177 --- /dev/null +++ b/lib/util/hasLoadingState.ts @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { TSESTree } from "@typescript-eslint/utils"; +import { hasNonEmptyProp } from "./hasNonEmptyProp"; + +/** + * Common prop names that indicate a loading state in FluentUI components + */ +const LOADING_STATE_PROPS = ["loading", "isLoading", "pending", "isPending", "busy", "isBusy"] as const; + +/** + * Determines if the component has any loading state indicator prop + * @param attributes - JSX attributes array + * @returns boolean indicating if component has loading state + */ +export const hasLoadingState = (attributes: TSESTree.JSXOpeningElement["attributes"]): boolean => { + return LOADING_STATE_PROPS.some(prop => hasNonEmptyProp(attributes, prop)); +}; + +/** + * Gets the specific loading prop that is present (for better error messages) + * @param attributes - JSX attributes array + * @returns string name of the loading prop found, or null if none + */ +export const getLoadingStateProp = (attributes: TSESTree.JSXOpeningElement["attributes"]): string | null => { + return LOADING_STATE_PROPS.find(prop => hasNonEmptyProp(attributes, prop)) ?? null; +}; diff --git a/tests/lib/rules/prefer-disabledfocusable-over-disabled.test.ts b/tests/lib/rules/prefer-disabledfocusable-over-disabled.test.ts new file mode 100644 index 0000000..ca217ea --- /dev/null +++ b/tests/lib/rules/prefer-disabledfocusable-over-disabled.test.ts @@ -0,0 +1,441 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Rule } from "eslint"; +import ruleTester from "./helper/ruleTester"; +import rule from "../../../lib/rules/prefer-disabledfocusable-over-disabled"; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +ruleTester.run("prefer-disabledfocusable-over-disabled", rule as unknown as Rule.RuleModule, { + valid: [ + // ✅ Correct usage: disabledFocusable with loading state + "", + "", + "", + "", + "", + + // ✅ Acceptable: disabled without loading state + "", + "", + "", + "", + "", + "", + + // ✅ Acceptable: loading state without disabled + "", + "", + "", + "", + "", + "Normal Combobox", + "Normal Dropdown", + "Normal SpinButton", + "Normal Slider", + "Normal DatePicker", + "Normal TimePicker", + "Normal Link", + "Normal Tab", + + // ✅ Test components with different casing variations + "", // lowercase (should not trigger - not a FluentUI component) + "", // uppercase (should not trigger - not a FluentUI component) + + // ✅ Cases where loading props are truly empty (null, undefined, empty string) + 'Submit', // loading is empty string + "", // loading is null + "", // loading is undefined + + // ✅ Cases where only one prop exists (no combination to trigger rule) + "", // only disabled, no loading + "", // only loading, no disabled + " true)()} />", // only loading, no disabled + "", // only disabled, no loading + + // ✅ JSXSpreadAttribute cases without both props + "", // only disabled, no loading + "", // only loading, no disabled + + // ✅ Cases where disabled has truly empty values + '', // disabled is empty string (should not trigger because disabled is empty) + "", // disabled is null (should not trigger because disabled is empty) + "" // disabled is undefined (should not trigger because disabled is empty) + ], + + invalid: [ + // ❌ Basic cases - test messageId: "preferDisabledFocusable" + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Boolean prop values (ALL boolean values are considered "non-empty" by hasNonEmptyProp) + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", // MOVED FROM VALID - false is still non-empty! + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Cases that were incorrectly in valid section (hasNonEmptyProp treats these as non-empty) + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", // 0 is considered non-empty + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Expression prop values + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Test all loading prop variations + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Multiple loading props (should still trigger) + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Test all applicable component types + { + code: "