react-responsive-modal icon indicating copy to clipboard operation
react-responsive-modal copied to clipboard

Close box doesn't work if CSS sets animation: none

Open timkingman opened this issue 3 years ago • 3 comments

Bug report

Describe the bug

I have some CSS to disable animations if the user has "reduce motion" turned on. (From https://twitter.com/justmarkup/status/974581638637142016 , https://davidwalsh.name/prefers-reduced-motion .)

@media (prefers-reduced-motion: reduce) {
  * {
    animation: none;
  }
}

This breaks the close button and the modal doesn't disappear. (onClose is called, I update state and pass open={false}, but the modal remains visible).

To Reproduce

The above CSS applies if the user has selected that accessibility option, and also seems to be set when using Remote Desktop. Here's a sandbox with animation: none set in CSS without the reduced-motion qualifier: https://codesandbox.io/s/react-responsive-modal-forked-m7zq6?file=/index.html

Open the modal (see it appear instantly because of animation: none), then click the close button. See nothing happen. React Dev Tools will show open updating to false, but the modal stays visible.

The sandbox also includes an alternative CSS rule that sets animation-duration very small but non-zero, from the comment on https://davidwalsh.name/prefers-reduced-motion#comment-514539 . This appears to work.

Expected behavior

Even with animations removed, I'd expect the close box to work. From the MDN docs on animationEnd, it's believable that this event just doesn't happen if there's no animation, so this might not be possible with the current Modal/Portal lifecycle methods.

System information

  • Version of react-responsive-modal: 6.0.1
  • Version of react: 16
  • Browser version: all

Additional context

I'm willing to believe that this my fault, and this animation: none rule is a bad idea and could break other components that build lifecycles around animation events and timing. See https://twitter.com/rikschennink/status/990688820763987969 The very small animation-duration version appears to work, so I may switch to that, or I may just remove this CSS that I copied without fully understanding its implications.

I spent several hours trying to figure this one out, so if nothing else, I hope this issue helps the next person who stumbles into this strange behavior of their own making.

timkingman avatar Mar 03 '21 18:03 timkingman

Thanks for the detailed issue, indeed it's a limitation of how the modal is built currently. It does expect animations to run in order to close the modal. As you said, the solution would be to run the animations with a very small but non-zero number. I can't think of another solution like this.

pradel avatar Mar 04 '21 08:03 pradel

You could add something like this to detect prefers-reduced-motion specifically.

const [reduceMotion, setReduceMotion] = useState(false);

// Update reduceMotion on media change
useEffect(()=>{
  const mediaQuery= window.matchMedia('(prefers-reduced-motion: reduce)');
  const updateMatch = (e: any) => setReduceMotion(e.matches);
  setReduceMotion(mediaQuery.matches);

  if (mediaQuery.addEventListener) {
      mediaQuery.addEventListener('change', updateMatch);
    } else {
      mediaQuery.addListener(updateMatch);
    }

    return () => {
      if (mediaQuery.removeEventListener) {
        mediaQuery.removeEventListener('change', updateMatch);
      } else {
        mediaQuery.removeListener(updateMatch);
      }
    };
}, []);

// If open is false and reduceMotion is true set showPortal to false
useEffect(()=>{
  if(open || !reduceMotion) return;

  setShowPortal(false);
},[open, reduceMotion])

Link2Twenty avatar Jun 18 '21 11:06 Link2Twenty

You could add something like this to detect prefers-reduced-motion specifically.

const [reduceMotion, setReduceMotion] = useState(false);

// Update reduceMotion on media change
useEffect(()=>{
  const mediaQuery= window.matchMedia('(prefers-reduced-motion: reduce)');
  const updateMatch = (e: any) => setReduceMotion(e.matches);
  setReduceMotion(mediaQuery.matches);

  if (mediaQuery.addEventListener) {
      mediaQuery.addEventListener('change', updateMatch);
    } else {
      mediaQuery.addListener(updateMatch);
    }

    return () => {
      if (mediaQuery.removeEventListener) {
        mediaQuery.removeEventListener('change', updateMatch);
      } else {
        mediaQuery.removeListener(updateMatch);
      }
    };
}, []);

// If open is false and reduceMotion is true set showPortal to false
useEffect(()=>{
  if(open || !reduceMotion) return;

  setShowPortal(false);
},[open, reduceMotion])

Goddamn, I thought my head was going to explode trying to figure out this bug. Thanks for the fix/workaround.

U-4-E-A avatar Jun 18 '21 22:06 U-4-E-A