Skip to content

Commit

Permalink
Button: Stabilize __experimentalIsFocusable prop (#62282)
Browse files Browse the repository at this point in the history
* Button: Stabilize `__experimentalIsFocusable` prop

* Add changelog

* Tweak changelog

Co-authored-by: Marin Atanasov <[email protected]>

* Replace `__experimentalIsFocusable` with `accessibleWhenDisabled` (#63107)

* Replace `__experimentalIsFocusable` with `accessibleWhenDisabled`

* Replace a few more remaining instances

* Revert "Replace a few more remaining instances"

This reverts commit 406ef77.

---------

Co-authored-by: Marin Atanasov <[email protected]>

* Fix new violation

---------

Co-authored-by: mirka <[email protected]>
Co-authored-by: tyxla <[email protected]>
Co-authored-by: stokesman <[email protected]>
  • Loading branch information
4 people authored Jul 4, 2024
1 parent d51d7e5 commit 2c62c6a
Show file tree
Hide file tree
Showing 48 changed files with 119 additions and 77 deletions.
4 changes: 2 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ const restrictedSyntax = [
const restrictedSyntaxComponents = [
{
selector:
'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="__experimentalIsFocusable"])) JSXAttribute[name.name="disabled"]',
'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="accessibleWhenDisabled"])) JSXAttribute[name.name="disabled"]',
message:
'`disabled` used without the `__experimentalIsFocusable` 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.)',
'`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.)',
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function DownloadableBlockListItem( { composite, item, onClick } ) {
<CompositeItem
render={
<Button
__experimentalIsFocusable
accessibleWhenDisabled
type="button"
role="option"
className="block-directory-downloadable-block-list-item"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default function InstallButton( { attributes, block, clientId } ) {
}
} )
}
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ isInstallingBlock }
isBusy={ isInstallingBlock }
variant="primary"
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-mover/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ const BlockMoverButton = forwardRef(
{ ...props }
onClick={ isDisabled ? null : onClick }
disabled={ isDisabled }
__experimentalIsFocusable
accessibleWhenDisabled
/>
<VisuallyHidden id={ descriptionId }>
{ getBlockMoverDescription(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ const CarouselNavigation = ( {
label={ __( 'Previous pattern' ) }
onClick={ handlePrevious }
disabled={ activeSlide === 0 }
__experimentalIsFocusable
accessibleWhenDisabled
/>
<Button
icon={ chevronRight }
label={ __( 'Next pattern' ) }
onClick={ handleNext }
disabled={ activeSlide === totalSlides - 1 }
__experimentalIsFocusable
accessibleWhenDisabled
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default function Pagination( {
onClick={ () => changePage( 1 ) }
disabled={ currentPage === 1 }
aria-label={ __( 'First page' ) }
__experimentalIsFocusable
accessibleWhenDisabled
>
<span>«</span>
</Button>
Expand All @@ -51,7 +51,7 @@ export default function Pagination( {
onClick={ () => changePage( currentPage - 1 ) }
disabled={ currentPage === 1 }
aria-label={ __( 'Previous page' ) }
__experimentalIsFocusable
accessibleWhenDisabled
>
<span></span>
</Button>
Expand All @@ -74,7 +74,7 @@ export default function Pagination( {
onClick={ () => changePage( currentPage + 1 ) }
disabled={ currentPage === numPages }
aria-label={ __( 'Next page' ) }
__experimentalIsFocusable
accessibleWhenDisabled
>
<span></span>
</Button>
Expand All @@ -84,7 +84,7 @@ export default function Pagination( {
disabled={ currentPage === numPages }
aria-label={ __( 'Last page' ) }
size="default"
__experimentalIsFocusable
accessibleWhenDisabled
>
<span>»</span>
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ export default function LinkPreview( {
isEmptyURL || showIconLabels ? '' : ': ' + value.url
) }
ref={ ref }
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ isEmptyURL }
size="compact"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function ConvertToLinksModal( { onClick, onClose, disabled } ) {
</Button>
<Button
variant="primary"
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ disabled }
onClick={ onClick }
>
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/page-list/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ export default function PageListEdit( {
<p>{ convertDescription }</p>
<Button
variant="primary"
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ ! hasResolvedPages }
onClick={ convertToNavigationLinks }
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function TitleModal( { areaLabel, onClose, onSubmit } ) {
<Button
variant="primary"
type="submit"
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ ! title.length }
>
{ __( 'Create' ) }
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
### Enhancements

- `ToolbarButton`: Deprecate `isDisabled` prop and merge with `disabled` ([#63101](https://github.com/WordPress/gutenberg/pull/63101)).
- `Button`: Stabilize `__experimentalIsFocusable` prop as `accessibleWhenDisabled` ([#62282](https://github.com/WordPress/gutenberg/pull/62282)).
- `ToolbarButton`: Always keep focusable when disabled ([#63102](https://github.com/WordPress/gutenberg/pull/63102)).

### Internal
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/autocomplete/autocompleter-ui.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function ListBox( {
id={ `components-autocomplete-item-${ instanceId }-${ option.key }` }
role="option"
aria-selected={ index === selectedIndex }
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ option.isDisabled }
className={ clsx(
'components-autocomplete__result',
Expand Down
13 changes: 13 additions & 0 deletions packages/components/src/button/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ The presence of a `href` prop determines whether an `anchor` element is rendered

Props not included in this set will be applied to the `a` or `button` element.

#### `accessibleWhenDisabled`: `boolean`

Whether to keep the button focusable when disabled.

In most cases, it is recommended to set this to `true`. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or by preventing focus from returning to a trigger element.

Learn more about the [focusability of disabled controls](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols) in the WAI-ARIA Authoring Practices Guide.

- Required: No
- Default: `false`

#### `children`: `ReactNode`

The button's children.
Expand All @@ -137,6 +148,8 @@ An accessible description for the button.

Whether the button is disabled. If `true`, this will force a `button` element to be rendered, even when an `href` is given.

In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.

- Required: No

#### `href`: `string`
Expand Down
10 changes: 6 additions & 4 deletions packages/components/src/button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { positionToPlacement } from '../popover/utils';
const disabledEventsOnDisabledButton = [ 'onMouseDown', 'onClick' ] as const;

function useDeprecatedProps( {
__experimentalIsFocusable,
isDefault,
isPrimary,
isSecondary,
Expand All @@ -43,7 +44,8 @@ function useDeprecatedProps( {
let computedSize = size;
let computedVariant = variant;

const newProps: { 'aria-pressed'?: boolean } = {
const newProps = {
accessibleWhenDisabled: __experimentalIsFocusable,
// @todo Mark `isPressed` as deprecated
'aria-pressed': isPressed,
};
Expand Down Expand Up @@ -91,6 +93,7 @@ export function UnforwardedButton(
) {
const {
__next40pxDefaultSize,
accessibleWhenDisabled,
isBusy,
isDestructive,
className,
Expand All @@ -106,7 +109,6 @@ export function UnforwardedButton(
size = 'default',
text,
variant,
__experimentalIsFocusable: isFocusable,
describedBy,
...buttonOrAnchorProps
} = useDeprecatedProps( props );
Expand Down Expand Up @@ -159,7 +161,7 @@ export function UnforwardedButton(
'has-icon': !! icon,
} );

const trulyDisabled = disabled && ! isFocusable;
const trulyDisabled = disabled && ! accessibleWhenDisabled;
const Tag = href !== undefined && ! trulyDisabled ? 'a' : 'button';
const buttonProps: ComponentPropsWithoutRef< 'button' > =
Tag === 'button'
Expand All @@ -177,7 +179,7 @@ export function UnforwardedButton(
const disableEventProps: {
[ key: string ]: ( event: MouseEvent ) => void;
} = {};
if ( disabled && isFocusable ) {
if ( disabled && accessibleWhenDisabled ) {
// In this case, the button will be disabled, but still focusable and
// perceivable by screen reader users.
buttonProps[ 'aria-disabled' ] = true;
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/button/stories/e2e/index.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const VariantStates: StoryFn< typeof Button > = (
{ ...props }
variant={ variant }
disabled
__experimentalIsFocusable
accessibleWhenDisabled
/>
<Button { ...props } variant={ variant } isBusy />
<Button { ...props } variant={ variant } isDestructive />
Expand Down
13 changes: 11 additions & 2 deletions packages/components/src/button/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ describe( 'Button', () => {
} );

it( 'should add only aria-disabled attribute when disabled and isFocusable are true', () => {
render( <Button disabled __experimentalIsFocusable /> );
render( <Button disabled accessibleWhenDisabled /> );
const button = screen.getByRole( 'button' );

expect( button ).toBeEnabled();
Expand Down Expand Up @@ -619,6 +619,15 @@ describe( 'Button', () => {
'mixed'
);
} );

it( 'should not break when the legacy __experimentalIsFocusable prop is passed', () => {
// eslint-disable-next-line no-restricted-syntax
render( <Button disabled __experimentalIsFocusable /> );
const button = screen.getByRole( 'button' );

expect( button ).toBeEnabled();
expect( button ).toHaveAttribute( 'aria-disabled' );
} );
} );

describe( 'static typing', () => {
Expand All @@ -638,7 +647,7 @@ describe( 'Button', () => {
{ /* @ts-expect-error */ }
<Button type="invalidtype" />
{ /* @ts-expect-error - although the runtime behavior will allow this to be an anchor, this is probably a mistake. */ }
<Button disabled __experimentalIsFocusable href="foo" />
<Button disabled accessibleWhenDisabled href="foo" />
</>;
} );
} );
37 changes: 27 additions & 10 deletions packages/components/src/button/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ type BaseButtonProps = {
* @default false
*/
__next40pxDefaultSize?: boolean;
/**
* Whether to keep the button focusable when disabled.
*
* In most cases, it is recommended to set this to `true`. Disabling a control without maintaining focusability
* can cause accessibility issues, by hiding their presence from screen reader users,
* or by preventing focus from returning to a trigger element.
*
* Learn more about the [focusability of disabled controls](https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#focusabilityofdisabledcontrols)
* in the WAI-ARIA Authoring Practices Guide.
*
* @default false
*/
accessibleWhenDisabled?: boolean;
/**
* The button's children.
*/
Expand Down Expand Up @@ -105,28 +118,24 @@ type BaseButtonProps = {
* 'link' (the link button styles)
*/
variant?: 'primary' | 'secondary' | 'tertiary' | 'link';
/**
* Whether to keep the button focusable when disabled.
*
* @default false
*/
__experimentalIsFocusable?: boolean;
};

type _ButtonProps = {
/**
* Whether the button is disabled.
* Whether the button is disabled. If `true`, this will force a `button` element
* to be rendered, even when an `href` is given.
*
* If `true`, this will force a `button` element to be rendered, even when an `href` is given.
* In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
*/
disabled?: boolean;
};

type AnchorProps = {
/**
* Whether the button is disabled.
* Whether the button is disabled. If `true`, this will force a `button` element
* to be rendered, even when an `href` is given.
*
* If `true`, this will force a `button` element to be rendered, even when an `href` is given.
* In most cases, it is recommended to also set the `accessibleWhenDisabled` prop to `true`.
*/
disabled?: false;
/**
Expand All @@ -140,6 +149,14 @@ type AnchorProps = {
};

export type DeprecatedButtonProps = {
/**
* Whether to keep the button focusable when disabled.
*
* @default false
* @deprecated Use the `accessibleWhenDisabled` prop instead.
* @ignore
*/
__experimentalIsFocusable?: boolean;
/**
* Gives the button a default style.
*
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/dropdown-menu/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {
? control.role
: 'menuitem'
}
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ control.isDisabled }
>
{ control.title }
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/font-size-picker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ const UnforwardedFontSizePicker = (
<FlexItem>
<Button
disabled={ isDisabled }
__experimentalIsFocusable
accessibleWhenDisabled
onClick={ () => {
onChange?.( undefined );
} }
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/range-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ function UnforwardedRangeControl(
<Button
className="components-range-control__reset"
// If the RangeControl itself is disabled, the reset button shouldn't be in the tab sequence.
__experimentalIsFocusable={ ! disabled }
accessibleWhenDisabled={ ! disabled }
disabled={ disabled || value === undefined }
variant="secondary"
size="small"
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/toolbar/toolbar-button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function UnforwardedToolbarButton(
className
) }
isPressed={ isActive }
__experimentalIsFocusable
accessibleWhenDisabled
data-toolbar-item
{ ...extraProps }
{ ...restProps }
Expand Down
2 changes: 1 addition & 1 deletion packages/dataviews/src/add-filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function AddFilter(
<DropdownMenu
trigger={
<Button
__experimentalIsFocusable
accessibleWhenDisabled
size="compact"
className="dataviews-filters-button"
variant="tertiary"
Expand Down
2 changes: 1 addition & 1 deletion packages/dataviews/src/item-actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ function CompactItemActions< Item >( {
size="compact"
icon={ moreVertical }
label={ __( 'Actions' ) }
__experimentalIsFocusable
accessibleWhenDisabled
disabled={ ! actions.length }
className="dataviews-all-actions-button"
/>
Expand Down
Loading

0 comments on commit 2c62c6a

Please sign in to comment.