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

Modal: Improve application of body class names #55430

Merged
merged 3 commits into from Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@
### Enhancements

- `DateTimePicker`: Adjustment of the dot position on DayButton and expansion of the button area. ([#55502](https://github.com/WordPress/gutenberg/pull/55502)).
- `Modal`: Improve application of body class names ([#55430](https://github.com/WordPress/gutenberg/pull/55430)).

### Experimental

Expand Down
32 changes: 17 additions & 15 deletions packages/components/src/modal/index.tsx
Expand Up @@ -43,12 +43,12 @@ import StyleProvider from '../style-provider';
import type { ModalProps } from './types';

// Used to track and dismiss the prior modal when another opens unless nested.
const level0Dismissers: MutableRefObject<
ModalProps[ 'onRequestClose' ] | undefined
>[] = [];
const ModalContext = createContext( level0Dismissers );
const ModalContext = createContext<
MutableRefObject< ModalProps[ 'onRequestClose' ] | undefined >[]
>( [] );
Comment on lines -46 to +48
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A level0Dismissers reference isn't needed anymore. It was added in #51602 only to maintain existing behavior. Now the effect for adding/removing body classes is nesting agnostic and simplified in that aspect.


let isBodyOpenClassActive = false;
// Used to track body class names applied while modals are open.
const bodyOpenClasses = new Map< string, number >();

function UnforwardedModal(
props: ModalProps,
Expand Down Expand Up @@ -146,7 +146,7 @@ function UnforwardedModal(
// one should remain open at a time and the list enables closing prior ones.
const dismissers = useContext( ModalContext );
// Used for the tracking and dismissing any nested modals.
const nestedDismissers = useRef< typeof level0Dismissers >( [] );
const nestedDismissers = useRef< typeof dismissers >( [] );

// Updates the stack tracking open modals at this level and calls
// onRequestClose for any prior and/or nested modals as applicable.
Expand All @@ -162,20 +162,22 @@ function UnforwardedModal(
};
}, [ dismissers ] );

const isLevel0 = dismissers === level0Dismissers;
// Adds/removes the value of bodyOpenClassName to body element.
useEffect( () => {
if ( ! isBodyOpenClassActive ) {
isBodyOpenClassActive = true;
document.body.classList.add( bodyOpenClassName );
}
const theClass = bodyOpenClassName;
const oneMore = 1 + ( bodyOpenClasses.get( theClass ) ?? 0 );
bodyOpenClasses.set( theClass, oneMore );
document.body.classList.add( bodyOpenClassName );
return () => {
if ( isLevel0 && dismissers.length === 0 ) {
document.body.classList.remove( bodyOpenClassName );
isBodyOpenClassActive = false;
const oneLess = bodyOpenClasses.get( theClass )! - 1;
if ( oneLess === 0 ) {
document.body.classList.remove( theClass );
bodyOpenClasses.delete( theClass );
} else {
bodyOpenClasses.set( theClass, oneLess );
}
};
}, [ bodyOpenClassName, dismissers, isLevel0 ] );
}, [ bodyOpenClassName ] );

// Calls the isContentScrollable callback when the Modal children container resizes.
useLayoutEffect( () => {
Expand Down
91 changes: 90 additions & 1 deletion packages/components/src/modal/test/index.tsx
Expand Up @@ -7,7 +7,7 @@ import userEvent from '@testing-library/user-event';
/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';
import { useEffect, useState } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -388,4 +388,93 @@ describe( 'Modal', () => {
expect( opener ).toHaveFocus();
} );
} );

describe( 'Body class name', () => {
const overrideClass = 'is-any-open';
const BodyClassDemo = () => {
const [ isAShown, setIsAShown ] = useState( false );
const [ isA1Shown, setIsA1Shown ] = useState( false );
const [ isBShown, setIsBShown ] = useState( false );
const [ isClassOverriden, setIsClassOverriden ] = useState( false );
useEffect( () => {
const toggles: ( e: KeyboardEvent ) => void = ( {
key,
metaKey,
} ) => {
if ( key === 'a' ) {
if ( metaKey ) return setIsA1Shown( ( v ) => ! v );
return setIsAShown( ( v ) => ! v );
}
if ( key === 'b' ) return setIsBShown( ( v ) => ! v );
if ( key === 'c' )
return setIsClassOverriden( ( v ) => ! v );
};
document.addEventListener( 'keydown', toggles );
return () =>
void document.removeEventListener( 'keydown', toggles );
}, [] );
return (
<>
{ isAShown && (
<Modal
bodyOpenClassName={
isClassOverriden ? overrideClass : 'is-A-open'
}
onRequestClose={ () => setIsAShown( false ) }
>
<p>Modal A contents</p>
{ isA1Shown && (
<Modal
title="Nested"
onRequestClose={ () =>
setIsA1Shown( false )
}
>
<p>Modal A1 contents</p>
</Modal>
) }
</Modal>
) }
{ isBShown && (
<Modal
bodyOpenClassName={
isClassOverriden ? overrideClass : 'is-B-open'
}
onRequestClose={ () => setIsBShown( false ) }
>
<p>Modal B contents</p>
</Modal>
) }
</>
);
};

it( 'is added and removed when modal opens and closes including when closed due to another modal opening', async () => {
const user = userEvent.setup();

const { baseElement } = render( <BodyClassDemo /> );

await user.keyboard( 'a' ); // Opens modal A.
expect( baseElement ).toHaveClass( 'is-A-open' );

await user.keyboard( 'b' ); // Opens modal B > closes modal A.
expect( baseElement ).toHaveClass( 'is-B-open' );
expect( baseElement ).not.toHaveClass( 'is-A-open' );

await user.keyboard( 'b' ); // Closes modal B.
expect( baseElement ).not.toHaveClass( 'is-B-open' );
} );

it( 'is removed even when prop changes while nested modal is open', async () => {
const user = userEvent.setup();

const { baseElement } = render( <BodyClassDemo /> );

await user.keyboard( 'a' ); // Opens modal A.
await user.keyboard( '{Meta>}a{/Meta}' ); // Opens nested modal.
await user.keyboard( 'c' ); // Changes `bodyOpenClassName`.
await user.keyboard( 'a' ); // Closes modal A.
expect( baseElement ).not.toHaveClass( 'is-A-open' );
} );
} );
} );