Skip to content

Commit

Permalink
Modal: Fix loss of focus when clicking outside (#52653)
Browse files Browse the repository at this point in the history
* Add unit test for focus return

* `Modal`: Fix loss of focus when clicking outside

* Add changelog entry

* Add comment
  • Loading branch information
stokesman committed Jul 25, 2023
1 parent e8564ed commit eea9e52
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 0 deletions.
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- `ColorPalette`, `BorderControl`: Don't hyphenate hex value in `aria-label` ([#52932](https://github.com/WordPress/gutenberg/pull/52932)).

### Bug Fix

- `Modal`: Fix loss of focus when clicking outside ([#52653](https://github.com/WordPress/gutenberg/pull/52653)).

## 25.4.0 (2023-07-20)

### Enhancements
Expand Down
16 changes: 16 additions & 0 deletions packages/components/src/modal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,15 @@ function UnforwardedModal(
[ hasScrolledContent ]
);

const onOverlayPress: React.PointerEventHandler< HTMLDivElement > = (
event
) => {
if ( event.target === event.currentTarget ) {
event.preventDefault();
onRequestClose( event );
}
};

return createPortal(
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<div
Expand All @@ -179,6 +188,13 @@ function UnforwardedModal(
overlayClassName
) }
onKeyDown={ handleEscapeKeyDown }
// Avoids loss of focus from clicking the overlay and also obviates
// `useFocusOutside` aside from cases of focus programmatically
// moving outside. TODO ideally both the hook and this handler
// won't be needed and one can be removed.
onPointerDown={
shouldCloseOnClickOutside ? onOverlayPress : undefined
}
>
<StyleProvider document={ document }>
<div
Expand Down
33 changes: 33 additions & 0 deletions packages/components/src/modal/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
import { render, screen, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';

/**
* Internal dependencies
*/
Expand Down Expand Up @@ -82,4 +87,32 @@ describe( 'Modal', () => {
await user.keyboard( '[Escape]' );
expect( onRequestClose ).toHaveBeenCalled();
} );

it( 'should return focus when dismissed by clicking outside', async () => {
const user = userEvent.setup();
const ReturnDemo = () => {
const [ isShown, setIsShown ] = useState( false );
return (
<div>
<button onClick={ () => setIsShown( true ) }>📣</button>
{ isShown && (
<Modal onRequestClose={ () => setIsShown( false ) }>
<p>Modal content</p>
</Modal>
) }
</div>
);
};
render( <ReturnDemo /> );

const opener = screen.getByRole( 'button' );
await user.click( opener );
const modalFrame = screen.getByRole( 'dialog' );
expect( modalFrame ).toHaveFocus();

// Disable reason: No semantic query can reach the overlay.
// eslint-disable-next-line testing-library/no-node-access
await user.click( modalFrame.parentElement! );
expect( opener ).toHaveFocus();
} );
} );

0 comments on commit eea9e52

Please sign in to comment.