Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Button: Stabilize __experimentalIsFocusable prop #62282

Merged
merged 7 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see this getting clearer now 👏

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
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', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for adding a test!

// 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
Loading