diff --git a/.eslintrc.js b/.eslintrc.js index e709db6c0d156b..79569225825084 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -108,16 +108,6 @@ const restrictedSyntax = [ }, ]; -/** `no-restricted-syntax` rules for components. */ -const restrictedSyntaxComponents = [ - { - selector: - 'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="accessibleWhenDisabled"])) JSXAttribute[name.name="disabled"]', - message: - '`disabled` used without the `accessibleWhenDisabled` prop. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or preventing focus from returning to a trigger element. (Ignore this error if you truly mean to disable.)', - }, -]; - module.exports = { root: true, extends: [ @@ -257,11 +247,8 @@ module.exports = { ], excludedFiles: [ '**/*.native.js' ], rules: { - 'no-restricted-syntax': [ - 'error', - ...restrictedSyntax, - ...restrictedSyntaxComponents, - ], + 'no-restricted-syntax': [ 'error', ...restrictedSyntax ], + '@wordpress/components-no-unsafe-button-disabled': 'error', }, }, { @@ -271,47 +258,9 @@ module.exports = { '**/*.@(native|ios|android).js', ], rules: { - 'no-restricted-syntax': [ - 'error', - ...restrictedSyntax, - ...restrictedSyntaxComponents, - // Temporary rules until we're ready to officially default to the new size. - ...[ - 'BorderBoxControl', - 'BorderControl', - 'BoxControl', - 'Button', - 'ComboboxControl', - 'CustomSelectControl', - - 'FontAppearanceControl', - 'FontFamilyControl', - 'FontSizePicker', - 'FormTokenField', - 'InputControl', - 'LetterSpacingControl', - 'LineHeightControl', - 'NumberControl', - 'RangeControl', - 'SelectControl', - 'TextControl', - 'ToggleGroupControl', - 'UnitControl', - ].map( ( componentName ) => ( { - // Falsy `__next40pxDefaultSize` without a non-default `size` prop. - selector: `JSXOpeningElement[name.name="${ componentName }"]:not(:has(JSXAttribute[name.name="__next40pxDefaultSize"][value.expression.value!=false])):not(:has(JSXAttribute[name.name="size"][value.value!="default"]))`, - message: - componentName + - ' should have the `__next40pxDefaultSize` prop when using the default size.', - } ) ), - { - // Falsy `__next40pxDefaultSize` without a `render` prop. - selector: - 'JSXOpeningElement[name.name="FormFileUpload"]:not(:has(JSXAttribute[name.name="__next40pxDefaultSize"][value.expression.value!=false])):not(:has(JSXAttribute[name.name="render"]))', - message: - 'FormFileUpload should have the `__next40pxDefaultSize` prop to opt-in to the new default size.', - }, - ], + 'no-restricted-syntax': [ 'error', ...restrictedSyntax ], + '@wordpress/components-no-unsafe-button-disabled': 'error', + '@wordpress/components-no-missing-40px-size-prop': 'error', }, }, { @@ -443,7 +392,6 @@ module.exports = { 'no-restricted-syntax': [ 'error', ...restrictedSyntax, - ...restrictedSyntaxComponents, { selector: ':matches(Literal[value=/--wp-admin-theme-/],TemplateElement[value.cooked=/--wp-admin-theme-/])', @@ -460,6 +408,22 @@ module.exports = { ], }, }, + { + // Override the @wordpress/components-* rules by adding the + // `checkLocalImports` flag, which adds the linting also to relative + // imports. + files: [ 'packages/components/src/**' ], + rules: { + '@wordpress/components-no-unsafe-button-disabled': [ + 'error', + { checkLocalImports: true }, + ], + // '@wordpress/components-no-missing-40px-size-prop': [ + // 'error', + // { checkLocalImports: true }, + // ], + }, + }, { files: [ 'packages/components/src/**' ], excludedFiles: [ 'packages/components/src/**/@(test|stories)/**' ], diff --git a/packages/block-editor/src/components/button-block-appender/index.js b/packages/block-editor/src/components/button-block-appender/index.js index 4cde8c26d75638..89b78f65ca5812 100644 --- a/packages/block-editor/src/components/button-block-appender/index.js +++ b/packages/block-editor/src/components/button-block-appender/index.js @@ -54,6 +54,8 @@ function ButtonBlockAppender( ); return ( + // Disable reason: There shouldn't be a case where this button is disabled but not visually hidden. + // eslint-disable-next-line @wordpress/components-no-unsafe-button-disabled + + + +``` + +Examples of **correct** code for this rule: + +```jsx +import { Button, InputControl } from '@wordpress/components'; + + + + + + +``` + +## FormFileUpload special case + +`FormFileUpload` can use either the `__next40pxDefaultSize` prop or the `render` prop to be considered valid: + +```jsx +import { FormFileUpload } from '@wordpress/components'; + +// Both are valid: + + } /> +``` + +## Options + +### checkLocalImports + +When set to `true`, the rule also checks components imported from relative paths. This is useful inside the `@wordpress/components` package itself, where components are imported via relative paths instead of `@wordpress/components`. + +```json +{ + "@wordpress/components-no-missing-40px-size-prop": [ + "error", + { "checkLocalImports": true } + ] +} +``` + +## Important notes + +- By default, this rule only applies to components imported from `@wordpress/components`. +- Components from other packages (like `@wordpress/ui`) or locally defined components with the same name are not affected. +- Aliased imports (e.g., `import { Button as WPButton }`) are correctly tracked. +- Components with a non-default `size` prop (e.g., `size="small"`, `size="compact"`) are exempt from this rule. +- Use the `checkLocalImports` option when linting inside the `@wordpress/components` package. diff --git a/packages/eslint-plugin/docs/rules/components-no-unsafe-button-disabled.md b/packages/eslint-plugin/docs/rules/components-no-unsafe-button-disabled.md new file mode 100644 index 00000000000000..4b74665cb18070 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/components-no-unsafe-button-disabled.md @@ -0,0 +1,52 @@ +# Disallow using disabled on Button without accessibleWhenDisabled (components-no-unsafe-button-disabled) + +Disabling a `Button` component without maintaining focusability can cause accessibility issues by hiding its presence from screen reader users, or preventing focus from returning to a trigger element. + +This rule enforces that `Button` components from `@wordpress/components` include the `accessibleWhenDisabled` prop when the `disabled` prop is set. + +## Rule details + +Examples of **incorrect** code for this rule: + +```jsx +import { Button } from '@wordpress/components'; + + + + +``` + +Examples of **correct** code for this rule: + +```jsx +import { Button } from '@wordpress/components'; + + + + + + + +``` + +## Options + +### checkLocalImports + +When set to `true`, the rule also checks `Button` components imported from relative paths. This is useful inside the `@wordpress/components` package itself, where components are imported via relative paths instead of `@wordpress/components`. + +```json +{ + "@wordpress/components-no-unsafe-button-disabled": [ + "error", + { "checkLocalImports": true } + ] +} +``` + +## Important notes + +- By default, this rule only applies to `Button` components imported from `@wordpress/components`. +- `Button` components from other packages (like `@wordpress/ui`) or locally defined components with the same name are not affected. +- Aliased imports (e.g., `import { Button as WPButton }`) are correctly tracked. +- Use the `checkLocalImports` option when linting inside the `@wordpress/components` package. diff --git a/packages/eslint-plugin/rules/__tests__/components-no-missing-40px-size-prop.js b/packages/eslint-plugin/rules/__tests__/components-no-missing-40px-size-prop.js new file mode 100644 index 00000000000000..88e0b6890ca94d --- /dev/null +++ b/packages/eslint-plugin/rules/__tests__/components-no-missing-40px-size-prop.js @@ -0,0 +1,346 @@ +import { RuleTester } from 'eslint'; +import rule from '../components-no-missing-40px-size-prop'; + +const ruleTester = new RuleTester( { + parserOptions: { + sourceType: 'module', + ecmaVersion: 6, + ecmaFeatures: { + jsx: true, + }, + }, +} ); + +ruleTester.run( 'components-no-missing-40px-size-prop', rule, { + valid: [ + // Component with __next40pxDefaultSize (boolean attribute) + { + code: ` + import { Button } from '@wordpress/components'; + } /> + `, + }, + // FormFileUpload with __next40pxDefaultSize + { + code: ` + import { FormFileUpload } from '@wordpress/components'; + + `, + }, + // Component with dynamic size prop (assumes it could be non-default) + { + code: ` + import { Button } from '@wordpress/components'; + } /> + `, + options: [ { checkLocalImports: true } ], + }, + ], + invalid: [], + } +); diff --git a/packages/eslint-plugin/rules/__tests__/components-no-unsafe-button-disabled.js b/packages/eslint-plugin/rules/__tests__/components-no-unsafe-button-disabled.js new file mode 100644 index 00000000000000..e27669b34823de --- /dev/null +++ b/packages/eslint-plugin/rules/__tests__/components-no-unsafe-button-disabled.js @@ -0,0 +1,235 @@ +import { RuleTester } from 'eslint'; +import rule from '../components-no-unsafe-button-disabled'; + +const ruleTester = new RuleTester( { + parserOptions: { + sourceType: 'module', + ecmaVersion: 6, + ecmaFeatures: { + jsx: true, + }, + }, +} ); + +ruleTester.run( 'components-no-unsafe-button-disabled', rule, { + valid: [ + // Button with both disabled and accessibleWhenDisabled + { + code: ` + import { Button } from '@wordpress/components'; + @@ -57,12 +52,6 @@ describe( 'Button', () => { it( 'can be enabled explicitly when loading', () => { render( - // Disabling because this lint rule was meant for the - // `@wordpress/components` Button, but is being applied here. - // TODO: rework the lint rule so that it checks the package - // where the Button comes from. - // TODO: Additional improvement in the original lint rule: only error if disabled=true? - // eslint-disable-next-line no-restricted-syntax @@ -76,11 +65,7 @@ describe( 'Button', () => { it( 'supports custom render prop while retaining the default focusable when disabled behavior', () => { render( - // Disabling because this lint rule was meant for the - // `@wordpress/components` Button, but is being applied here. - // TODO: rework the lint rule so that it checks the package - // where the Button comes from. - // eslint-disable-next-line jsx-a11y/anchor-has-content, no-restricted-syntax + // eslint-disable-next-line jsx-a11y/anchor-has-content