From 0f873b39314bb018d98fc791c30a85271bc53540 Mon Sep 17 00:00:00 2001 From: Sidharth Sharma Date: Wed, 17 Sep 2025 23:25:08 -0400 Subject: [PATCH 1/4] Add capability to prefer disabledFocusable for components with loading state. --- .../prefer-disabledfocusable-over-disabled.md | 139 ++++++++++++++ .../disabledFocusableComponents.ts | 36 ++++ lib/index.ts | 2 + lib/rules/index.ts | 1 + .../prefer-disabledfocusable-over-disabled.ts | 88 +++++++++ lib/util/hasLoadingState.ts | 28 +++ ...er-disabledfocusable-over-disabled.test.ts | 169 ++++++++++++++++++ 7 files changed, 463 insertions(+) create mode 100644 docs/rules/prefer-disabledfocusable-over-disabled.md create mode 100644 lib/applicableComponents/disabledFocusableComponents.ts create mode 100644 lib/rules/prefer-disabledfocusable-over-disabled.ts create mode 100644 lib/util/hasLoadingState.ts create mode 100644 tests/lib/rules/prefer-disabledfocusable-over-disabled.test.ts diff --git a/docs/rules/prefer-disabledfocusable-over-disabled.md b/docs/rules/prefer-disabledfocusable-over-disabled.md new file mode 100644 index 0000000..05b2020 --- /dev/null +++ b/docs/rules/prefer-disabledfocusable-over-disabled.md @@ -0,0 +1,139 @@ +# Prefer 'disabledFocusable' over 'disabled' when component has loading state (`@microsoft/fluentui-jsx-a11y/prefer-disabledfocusable-over-disabled`) + +⚠️ This rule is enabled 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 bbf4bfc..297ea85 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -37,6 +37,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", @@ -75,6 +76,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 bfdbe7b..bd8ee87 100644 --- a/lib/rules/index.ts +++ b/lib/rules/index.ts @@ -35,3 +35,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..93a5b4c --- /dev/null +++ b/tests/lib/rules/prefer-disabledfocusable-over-disabled.test.ts @@ -0,0 +1,169 @@ +// 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 + "", + "", + "", + "", + + // ✅ Acceptable: neither disabled nor loading + "", + "", + "", + + // ✅ Non-applicable components (don't support disabledFocusable) + "
Content
", + "", + "Text", + + // ✅ Edge case: both disabledFocusable and disabled present (different rule concern) + "", + + // ✅ Complex expressions + "", + "", + + // ✅ All supported loading props + "", + "", + "" + ], + + invalid: [ + // ❌ Basic cases + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Boolean prop values + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Expression prop values + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + + // ❌ Different loading prop variations + { + 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: "" + }, + + // ❌ Different component types + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + { + code: "", + "Normal Combobox", + "Normal Dropdown", + "Normal SpinButton", + "Normal Slider", + "Normal DatePicker", + "Normal TimePicker", + "Normal Link", + "Normal Tab" ], invalid: [ - // ❌ Basic cases + // ❌ Basic cases - test messageId: "preferDisabledFocusable" { code: "", errors: [{ messageId: "preferDisabledFocusable" }], @@ -71,6 +134,14 @@ ruleTester.run("prefer-disabledfocusable-over-disabled", rule as unknown as Rule output: "" }, + // ❌ Test case where getLoadingStateProp returns null (should use preferDisabledFocusableGeneric) + // This shouldn't happen in practice but tests the fallback + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, + // ❌ Boolean prop values { code: "", @@ -95,21 +166,36 @@ ruleTester.run("prefer-disabledfocusable-over-disabled", rule as unknown as Rule output: "" }, - // ❌ Different loading prop variations + // ❌ 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: "", + code: "", errors: [{ messageId: "preferDisabledFocusable" }], - output: "" + output: "" }, { - code: "", + code: "", errors: [{ messageId: "preferDisabledFocusable" }], - output: "" + output: "" }, // ❌ Multiple loading props (should still trigger) @@ -118,22 +204,42 @@ ruleTester.run("prefer-disabledfocusable-over-disabled", rule as unknown as Rule errors: [{ messageId: "preferDisabledFocusable" }], output: "" }, + { + code: "", + errors: [{ messageId: "preferDisabledFocusable" }], + output: "" + }, - // ❌ Different component types + // ❌ Test all applicable component types { - code: "", + code: "