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

useFocusOutside: do not fire callback when focusing on an iframe inside the wrapper #45349

Closed
wants to merge 6 commits into from
Closed
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: 3 additions & 1 deletion packages/components/src/modal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ function UnforwardedModal(
const focusOnMountRef = useFocusOnMount( focusOnMount );
const constrainedTabbingRef = useConstrainedTabbing();
const focusReturnRef = useFocusReturn();
const focusOutsideProps = useFocusOutside( onRequestClose );
const { ref: focusOutsideRef, ...focusOutsideProps } =
useFocusOutside( onRequestClose );

const [ hasScrolledContent, setHasScrolledContent ] = useState( false );

Expand Down Expand Up @@ -147,6 +148,7 @@ function UnforwardedModal(
constrainedTabbingRef,
focusReturnRef,
focusOnMountRef,
focusOutsideRef,
] ) }
role={ role }
aria-label={ contentLabel }
Expand Down
38 changes: 38 additions & 0 deletions packages/components/src/modal/stories/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
* External dependencies
*/
import type { ComponentStory, ComponentMeta } from '@storybook/react';
import type { FC } from 'react';
import { createPortal } from 'react-dom';

/**
* WordPress dependencies
Expand Down Expand Up @@ -41,6 +43,29 @@ const meta: ComponentMeta< typeof Modal > = {
};
export default meta;

const IFrame: FC< { title?: string; width: number; height: number } > = ( {
children,
...props
} ) => {
const [ contentRef, setContentRef ] = useState< HTMLIFrameElement | null >(
null
);
const mountNode = contentRef?.contentWindow?.document?.body;

return (
<iframe
title="test-iframe"
{ ...props }
ref={ setContentRef }
onBlur={ () => console.log( 'iframe blurred' ) }
onFocus={ () => console.log( 'iframe focused' ) }
tabIndex={ -1 }
>
{ mountNode && createPortal( children, mountNode ) }
</iframe>
);
};

const Template: ComponentStory< typeof Modal > = ( {
onRequestClose,
...args
Expand Down Expand Up @@ -75,6 +100,19 @@ const Template: ComponentStory< typeof Modal > = ( {
anim id est laborum.
</p>

<button>Ciao</button>

<iframe
title="Example 1"
width="300"
height="200"
src="https://www.openstreetmap.org/export/embed.html?bbox=-0.004017949104309083%2C51.47612752641776%2C0.00030577182769775396%2C51.478569861898606&layer=mapnik"
/>

<IFrame title="Example 2" width={ 300 } height={ 200 }>
<button>Ciao</button>
</IFrame>

<Button variant="secondary" onClick={ closeModal }>
Close Modal
</Button>
Expand Down
3 changes: 2 additions & 1 deletion packages/compose/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

### Internal

- `useDisabled`: Refactor the component to rely on the HTML `inert` attribute.
- `useDisabled`: Refactor the component to rely on the HTML `inert` attribute ([#44865](https://github.com/WordPress/gutenberg/pull/44865)).
- `useFocusOutside`: Refactor the hook to TypeScript, rewrite tests using modern RTL and jest features ([#45317](https://github.com/WordPress/gutenberg/pull/45317)).

## 5.18.0 (2022-10-19)

Expand Down
25 changes: 15 additions & 10 deletions packages/compose/src/hooks/use-dialog/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import useFocusOnMount from '../use-focus-on-mount';
import useFocusReturn from '../use-focus-return';
import useFocusOutside from '../use-focus-outside';
import useMergeRefs from '../use-merge-refs';
import type { FocusOutsideReturnValue } from '../use-focus-outside';

type DialogOptions = {
focusOnMount?: Parameters< typeof useFocusOnMount >[ 0 ];
Expand All @@ -35,7 +34,7 @@ type DialogOptions = {

type useDialogReturn = [
RefCallback< HTMLElement >,
FocusOutsideReturnValue & Pick< HTMLElement, 'tabIndex' >
ReturnType< typeof useFocusOutside > & Pick< HTMLElement, 'tabIndex' >
];

/**
Expand All @@ -55,15 +54,20 @@ function useDialog( options: DialogOptions ): useDialogReturn {
const constrainedTabbingRef = useConstrainedTabbing();
const focusOnMountRef = useFocusOnMount( options.focusOnMount );
const focusReturnRef = useFocusReturn();
const focusOutsideProps = useFocusOutside( ( event ) => {
// This unstable prop is here only to manage backward compatibility
// for the Popover component otherwise, the onClose should be enough.
if ( currentOptions.current?.__unstableOnClose ) {
currentOptions.current.__unstableOnClose( 'focus-outside', event );
} else if ( currentOptions.current?.onClose ) {
currentOptions.current.onClose();
const { ref: focusOutsideRef, ...focusOutsideProps } = useFocusOutside(
( event ) => {
// This unstable prop is here only to manage backward compatibility
// for the Popover component otherwise, the onClose should be enough.
if ( currentOptions.current?.__unstableOnClose ) {
currentOptions.current.__unstableOnClose(
'focus-outside',
event
);
} else if ( currentOptions.current?.onClose ) {
currentOptions.current.onClose();
}
}
} );
);
const closeOnEscapeRef = useCallback( ( node: HTMLElement ) => {
if ( ! node ) {
return;
Expand All @@ -88,6 +92,7 @@ function useDialog( options: DialogOptions ): useDialogReturn {
options.focusOnMount !== false ? focusReturnRef : null,
options.focusOnMount !== false ? focusOnMountRef : null,
closeOnEscapeRef,
focusOutsideRef,
] ),
{
...focusOutsideProps,
Expand Down
193 changes: 0 additions & 193 deletions packages/compose/src/hooks/use-focus-outside/index.js

This file was deleted.

Loading