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

SLVUU-123: Fix Toast Animation #125

Closed
wants to merge 2 commits into from

Conversation

pling-scottlogic
Copy link

Description

Fixes the entry animation for toast notifications.

Change List

  • Add nominal timeout around the function that puts the notification on screen
  • Refactor: remove unnecessary CSS class

Testing

  • Tested manually on both showcase and sample app
  • Manual testing performed in Firefox, Edge and Chrome

Test Evidence

Before

Image

After

SLVUU123-toast-animation-fixed

Implementation Notes

The setRight function changes a CSS property, resulting in the notification moving into the screen from the right. Despite being defined with zero time, the timeout pushes the setRight call from the stack to the task queue. This is enough to create a distinction between the offscreen and onscreen positions, so that the transition effect can be seen.

Copy link

@cfisher-scottlogic cfisher-scottlogic left a comment

Choose a reason for hiding this comment

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

Part of me wonders whether there's a more 'React-y' way to do this, with hooks like useMemo or useCallback. Or maybe even an async/await. These solutions would make it feel less 'arbitrary', like 'hey wait for this specific thing I'm dependent on' rather than just 'wait'.

But this is entirely functional and clean. Well done for figuring it out! 🙂

@cfisher-scottlogic
Copy link

cfisher-scottlogic commented Nov 29, 2023

Part of me wonders whether there's a more 'React-y' way to do this, with hooks like useMemo or useCallback. Or maybe even an async/await. These solutions would make it feel less 'arbitrary', like 'hey wait for this specific thing I'm dependent on' rather than just 'wait'.

But this is entirely functional and clean. Well done for figuring it out! 🙂

E.g. A useEffect approach. I dislike having one useEffect depend on another useEffect in this way though. It feels like there should be a cleaner solution.

  const [toastHidden, setToastHidden] = useState(false);
  const [right, setRight] = useState<number>();

  useEffect(() => {
    setRight(-toastWidth - toastContainerRightPadding);
    setToastHidden(true);
  }, []);

  useEffect(() => {
    if (toastHidden) {
      setRight(toastContainerRightPadding);
    }

    if (animated) {
      setTimeout(
        () => setRight(-toastWidth - toastContainerRightPadding),
        toastDisplayDuration + horizontalTransitionDuration
      );
    }
  }, [toastHidden, animated]);

@pling-scottlogic
Copy link
Author

External PR

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

Successfully merging this pull request may close these issues.

Toast notifications not animating on entry
3 participants