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

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

Open
timkingman opened this issue Mar 3, 2021 · 3 comments
Open

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

timkingman opened this issue Mar 3, 2021 · 3 comments
Labels

Comments

@timkingman
Copy link

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 timkingman added the bug label Mar 3, 2021
@pradel
Copy link
Owner

pradel commented Mar 4, 2021

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.

@Link2Twenty
Copy link

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])

@U-4-E-A
Copy link

U-4-E-A commented Jun 18, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants