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

Allow Modal to place focus on first element within contents via new API #54590

Merged
merged 14 commits into from
Sep 21, 2023
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/block-rename-ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ function RenameModal( { blockName, originalBlockName, onClose, onSave } ) {
aria={ {
describedby: dialogDescription,
} }
focusOnMount="firstContentElement"
>
<p id={ dialogDescription }>
{ __( 'Enter a custom name for this block.' ) }
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Enhancements

- Add new option `firstContentElement` to Modal's `focusOnMount` prop to allow consumers to focus the first element within the Modal's **contents** ([#54590](https://github.com/WordPress/gutenberg/pull/54590)).
- `Notice`: Improve accessibility by adding visually hidden text to clarify what a notice text is about and the notice type (success, error, warning, info) ([#54498](https://github.com/WordPress/gutenberg/pull/54498)).
- Making Circular Option Picker a `listbox`. Note that while this changes some public API, new props are optional, and currently have default values; this will change in another patch ([#52255](https://github.com/WordPress/gutenberg/pull/52255)).
- `ToggleGroupControl`: Rewrite backdrop animation using framer motion shared layout animations, add better support for controlled and uncontrolled modes ([#50278](https://github.com/WordPress/gutenberg/pull/50278)).
Expand Down
8 changes: 7 additions & 1 deletion packages/components/src/modal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,16 @@ Titles are required for accessibility reasons, see `aria.labelledby` and `title`

- Required: No

#### `focusOnMount`: `boolean | 'firstElement'`
#### `focusOnMount`: `boolean | 'firstElement'` | 'firstContentElement'

If this property is true, it will focus the first tabbable element rendered in the modal.

If this property is false, focus will not be transferred and it is the responsibility of the consumer to ensure accessible focus management.

If set to `firstElement` focus will be placed on the first tabbable element anywhere within the Modal.

If set to `firstContentElement` focus will be placed on the first tabbable element within the Modal's **content** (i.e. children). Note that it is the responsibility of the consumer to ensure there is at least one tabbable element within the children **or the focus will be lost**.

- Required: No
- Default: `true`

Expand Down
30 changes: 27 additions & 3 deletions packages/components/src/modal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,23 @@ function UnforwardedModal(
} = props;

const ref = useRef< HTMLDivElement >();

const instanceId = useInstanceId( Modal );
const headingId = title
? `components-modal-header-${ instanceId }`
: aria.labelledby;
const focusOnMountRef = useFocusOnMount( focusOnMount );

// The focus hook does not support 'firstContentElement' but this is a valid
// value for the Modal's focusOnMount prop. The following code ensures the focus
// hook will focus the first focusable node within the element to which it is applied.
// When `firstContentElement` is passed as the value of the focusOnMount prop,
// the focus hook is applied to the Modal's content element.
// Otherwise, the focus hook is applied to the Modal's ref. This ensures that the
// focus hook will focus the first element in the Modal's **content** when
// `firstContentElement` is passed.
const focusOnMountRef = useFocusOnMount(
focusOnMount === 'firstContentElement' ? 'firstElement' : focusOnMount
);
getdave marked this conversation as resolved.
Show resolved Hide resolved
const constrainedTabbingRef = useConstrainedTabbing();
const focusReturnRef = useFocusReturn();
const focusOutsideProps = useFocusOutside( onRequestClose );
Expand Down Expand Up @@ -223,7 +235,9 @@ function UnforwardedModal(
ref={ useMergeRefs( [
constrainedTabbingRef,
focusReturnRef,
focusOnMountRef,
focusOnMount !== 'firstContentElement'
? focusOnMountRef
: null,
] ) }
role={ role }
aria-label={ contentLabel }
Expand Down Expand Up @@ -283,7 +297,17 @@ function UnforwardedModal(
) }
</div>
) }
<div ref={ childrenContainerRef }>{ children }</div>

<div
ref={ useMergeRefs( [
childrenContainerRef,
focusOnMount === 'firstContentElement'
? focusOnMountRef
: null,
] ) }
>
{ children }
</div>
</div>
</div>
</StyleProvider>
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/modal/stories/index.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ const meta: Meta< typeof Modal > = {
control: { type: null },
},
focusOnMount: {
control: { type: 'boolean' },
options: [ true, false, 'firstElement', 'firstContentElement' ],
control: { type: 'select' },
},
role: {
control: { type: 'text' },
Expand Down
124 changes: 124 additions & 0 deletions packages/components/src/modal/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { useState } from '@wordpress/element';
* Internal dependencies
*/
import Modal from '../';
import type { ModalProps } from '../types';

const noop = () => {};

Expand Down Expand Up @@ -236,4 +237,127 @@ describe( 'Modal', () => {
screen.getByText( 'A sweet button', { selector: 'button' } )
).toBeInTheDocument();
} );

describe( 'Focus handling', () => {
let originalGetClientRects: () => DOMRectList;

const FocusMountDemo = ( {
focusOnMount,
}: Pick< ModalProps, 'focusOnMount' > ) => {
const [ isShown, setIsShown ] = useState( false );
return (
<>
<button onClick={ () => setIsShown( true ) }>
Toggle Modal
</button>
{ isShown && (
<Modal
focusOnMount={ focusOnMount }
onRequestClose={ () => setIsShown( false ) }
>
<p>Modal content</p>
<a href="https://wordpress.org">
First Focusable Content Element
</a>

<a href="https://wordpress.org">
Another Focusable Content Element
</a>
</Modal>
) }
</>
);
};

beforeEach( () => {
/**
* The test environment does not have a layout engine, so we need to mock
* the getClientRects method. This ensures that the focusable elements can be
* found by the `focusOnMount` logic which depends on layout information
* to determine if the element is visible or not.
* See https://github.com/WordPress/gutenberg/blob/trunk/packages/dom/src/focusable.js#L55-L61.
*/
// @ts-expect-error We're not trying to comply to the DOM spec, only mocking
window.HTMLElement.prototype.getClientRects = function () {
return [ 'trick-jsdom-into-having-size-for-element-rect' ];
};
} );

afterEach( () => {
// Restore original HTMLElement prototype.
// See beforeEach for details.
window.HTMLElement.prototype.getClientRects =
originalGetClientRects;
} );

it( 'should focus the Modal dialog by default when `focusOnMount` prop is not provided', async () => {
const user = userEvent.setup();

render( <FocusMountDemo /> );

const opener = screen.getByRole( 'button', {
name: 'Toggle Modal',
} );

await user.click( opener );

expect( screen.getByRole( 'dialog' ) ).toHaveFocus();
} );

it( 'should focus the Modal dialog when `true` passed as value for `focusOnMount` prop', async () => {
const user = userEvent.setup();

render( <FocusMountDemo focusOnMount={ true } /> );

const opener = screen.getByRole( 'button', {
name: 'Toggle Modal',
} );

await user.click( opener );

expect( screen.getByRole( 'dialog' ) ).toHaveFocus();
} );

it( 'should focus the first focusable element in the contents (if found) when `firstContentElement` passed as value for `focusOnMount` prop', async () => {
const user = userEvent.setup();

render( <FocusMountDemo focusOnMount="firstContentElement" /> );

const opener = screen.getByRole( 'button' );

await user.click( opener );

expect(
screen.getByText( 'First Focusable Content Element' )
).toHaveFocus();
} );

it( 'should focus the first element anywhere within the Modal when `firstElement` passed as value for `focusOnMount` prop', async () => {
const user = userEvent.setup();

render( <FocusMountDemo focusOnMount="firstElement" /> );

const opener = screen.getByRole( 'button' );

await user.click( opener );

expect(
screen.getByRole( 'button', { name: 'Close' } )
).toHaveFocus();
} );

it( 'should not move focus when `false` passed as value for `focusOnMount` prop', async () => {
const user = userEvent.setup();

render( <FocusMountDemo focusOnMount={ false } /> );

const opener = screen.getByRole( 'button', {
name: 'Toggle Modal',
} );

await user.click( opener );

expect( opener ).toHaveFocus();
} );
} );
} );
4 changes: 3 additions & 1 deletion packages/components/src/modal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ export type ModalProps = {
*
* @default true
*/
focusOnMount?: Parameters< typeof useFocusOnMount >[ 0 ];
focusOnMount?:
| Parameters< typeof useFocusOnMount >[ 0 ]
| 'firstContentElement';
/**
* Elements that are injected into the modal header to the left of the close button (if rendered).
* Hidden if `__experimentalHideHeader` is `true`.
Expand Down
12 changes: 7 additions & 5 deletions test/e2e/specs/editor/various/block-renaming.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,23 @@ test.describe( 'Block Renaming', () => {
name: 'Rename',
} );

// Check focus is transferred into modal.
await expect( renameModal ).toBeFocused();

// Check the Modal is perceivable.
await expect( renameModal ).toBeVisible();

const nameInput = renameModal.getByRole( 'textbox', {
name: 'Block name',
} );

// Check focus is transferred into the input within the Modal.
await expect( nameInput ).toBeFocused();

const saveButton = renameModal.getByRole( 'button', {
name: 'Save',
type: 'submit',
} );

await expect( saveButton ).toBeDisabled();

const nameInput = renameModal.getByLabel( 'Block name' );

await expect( nameInput ).toHaveAttribute( 'placeholder', 'Group' );

await nameInput.fill( 'My new name' );
Expand Down
Loading