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
6 changes: 5 additions & 1 deletion packages/components/src/modal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,14 @@ 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 set to `firstElement` it will focus the first focusable element anywhere within the Modal.

If set to `firstContentElement` it will focus the first focusable element within the Modal's **content** (i.e. children).
getdave marked this conversation as resolved.
Show resolved Hide resolved

- 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
140 changes: 140 additions & 0 deletions packages/components/src/modal/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -236,4 +236,144 @@ describe( 'Modal', () => {
screen.getByText( 'A sweet button', { selector: 'button' } )
).toBeInTheDocument();
} );

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

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 first focusable element in the contents (if found) when `firstContentElement` passed as value for `focusOnMount` prop', async () => {
getdave marked this conversation as resolved.
Show resolved Hide resolved
const user = userEvent.setup();

const FocusMountDemo = () => {
const [ isShown, setIsShown ] = useState( false );
return (
<>
<button onClick={ () => setIsShown( true ) }>📣</button>
{ isShown && (
<Modal
focusOnMount="firstContentElement"
onRequestClose={ () => setIsShown( false ) }
>
<p>Modal content</p>
<a
href="https://wordpress.org"
data-testid="first-focusable-element"
>
First Focusable Element
</a>

<a href="https://wordpress.org">
Another Focusable Element
</a>
</Modal>
) }
</>
);
};
getdave marked this conversation as resolved.
Show resolved Hide resolved

render( <FocusMountDemo /> );

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

await user.click( opener );

expect(
screen.getByTestId( 'first-focusable-element' )
).toHaveFocus();
getdave marked this conversation as resolved.
Show resolved Hide resolved
} );

it( 'should focus the Modal dialog when `true` passed as value for `focusOnMount` prop', async () => {
const user = userEvent.setup();
const FocusMountDemo = () => {
const [ isShown, setIsShown ] = useState( false );
return (
<>
<button onClick={ () => setIsShown( true ) }>📣</button>
{ isShown && (
<Modal
focusOnMount
onRequestClose={ () => setIsShown( false ) }
>
<p>Modal content</p>
<a
href="https://wordpress.org"
data-testid="first-focusable-element"
>
First Focusable Element
</a>

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

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

await user.click( opener );

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

it( 'should not move focus when `false` passed as value for `focusOnMount` prop', async () => {
const user = userEvent.setup();
const FocusMountDemo = () => {
const [ isShown, setIsShown ] = useState( false );
return (
<>
<button onClick={ () => setIsShown( true ) }>📣</button>
{ isShown && (
<Modal
focusOnMount={ false }
onRequestClose={ () => setIsShown( false ) }
>
<p>Modal content</p>
<a
href="https://wordpress.org"
data-testid="first-focusable-element"
>
First Focusable Element
</a>

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

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

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
10 changes: 5 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,21 @@ 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.getByLabel( 'Block name' );
getdave marked this conversation as resolved.
Show resolved Hide resolved

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