2

I have a modal dialog that I want to close if the user clicks outside of the modal. I have written the following useEffect code but I run into following issue:

The modal dialog contains a number of children (React Nodes) and those children might change (e.g. the user deletes an entry of a list). Those interactions trigger my onClick method but as the clicked list item has been removed from the modal, the modal closes even though the click was within the modal.

I thought adding [ children ] at the second parameter for useEffect would cleanup the old effect event listener quick enough that the method does not run again but this is not the case.

I handled the same issue in a class component with a ignoreNextClick-state but there must be a cleaner solution, right?

    useEffect( () => {
        const onClick = ( event ) => {
            const menu = document.getElementById( 'singleton-modal' );
            if ( !menu ) return;

            // do not close menu if user clicked inside
            const targetInMenu = menu.contains( event.target );
            const targetIsMenu = menu === event.target;
            if ( targetInMenu || targetIsMenu ) return;

            onCloseModal();
        };

        window.addEventListener( 'click', onClick, false );

        return () => window.removeEventListener( 'click', onClick, false );
    }, [ children ] );
skyboyer
  • 22,209
  • 7
  • 57
  • 64
Andre
  • 4,185
  • 5
  • 22
  • 32

2 Answers2

2

I found a solution that does not require any sort of storing old props.

The useEffect call looks like this:

useEffect( () => {
        const onClickOutside = () => onCloseModal();
        window.addEventListener( 'click', onClickOutside, false );
        return () => window.removeEventListener( 'click', onClickOutside );
    }, [] );

Adding the following click listener to the modal directly will stop the window click-listener from being called if the user clicked inside the modal.

<div
    className={`modal ${ classes }`}
    onClick={event => event.stopPropagation()}
    role="presentation"
>
   {children}
</div>`

I also added the role presentation to make the modal more accessible and aria-conform.

Andre
  • 4,185
  • 5
  • 22
  • 32
0

You can check parent of modal from the event.target. If the current target is within the modal then return.

You can use closest to do that.

See the following solution.

...

    if (event.target.closest( '.singleton-modal' ) || event.target.classList.contains('singleton-modal')) {
       return;
    }

...
Radonirina Maminiaina
  • 6,958
  • 4
  • 33
  • 60
  • Same issue as event.target has been removed from the DOM already (I guess this is the issue) event.target.closest does not find a modal in the target hierarchy anymore. – Andre Jun 24 '19 at 14:08