Skip to content

Commit

Permalink
Allow Modal to place focus on first element within contents via new A…
Browse files Browse the repository at this point in the history
…PI (#54590)

* Add new firstContentElement variation to Modal

* Update Story

* Update e2e test

* Add focus tests

* Conditionally add ref

* Provide comment to explain unusual code

* Remove extra div element

* Update docs

* Tweak code comment documentation

* Add test for firstElement

* Refactor tests

* Prefer getByRole

* Improve README with additional context
  • Loading branch information
getdave authored Sep 21, 2023
1 parent 8385ae7 commit a0d00ca
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 11 deletions.
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 @@ -11,6 +11,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
);
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

1 comment on commit a0d00ca

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in a0d00ca.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6260063425
📝 Reported issues:

Please sign in to comment.