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

feat: added flexibility for timer #360

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Flaain
Copy link

@Flaain Flaain commented Mar 4, 2024

Issue 😱:

Closes -

What has been done βœ…:

Hello, i was trying to recreate Sonner by myself and found out some unnecessary code in useEffect timer:

React.useEffect(() => {
    if ((toast.promise && toastType === 'loading') || toast.duration === Infinity || toast.type === 'loading') return;
    let timeoutId: NodeJS.Timeout;
    let remainingTime = duration;

    // Pause the timer on each hover
    const pauseTimer = () => {
      if (lastCloseTimerStartTimeRef.current < closeTimerStartTimeRef.current) {
        // Get the elapsed time since the timer started
        const elapsedTime = new Date().getTime() - closeTimerStartTimeRef.current;

        remainingTime = remainingTime - elapsedTime;
      }

      lastCloseTimerStartTimeRef.current = new Date().getTime();
    };

    const startTimer = () => {
      closeTimerStartTimeRef.current = new Date().getTime();

      // Let the toast know it has started
      timeoutId = setTimeout(() => {
        toast.onAutoClose?.(toast);
        deleteToast();
      }, remainingTime);
    };

    if (expanded || interacting || (pauseWhenPageIsHidden && isDocumentHidden)) {
      pauseTimer();
    } else {
      startTimer();
    }

    return () => clearTimeout(timeoutId);
  }, [dependencies]);

What unnecessary code this useEffect have?:

  1. We don't need let remainingTime = duration and recalculate logic for remainingTime because we don't save remainingTime between rerenders. In this case we can just use duration prop for setTimeout nothing will change.
  2. Also don't need let timeoutId: NodeJS.Timeout;, closeTimerStartTimeRef and lastCloseTimerStartTimeRef we can contain this in timerRef.

What did i do?:

  1. Added optional prop that defaults to false recalculateRemainingTime = false.
  2. Instead of 2 lastCloseTimerStartTimeRef and closeTimerStartTimeRef create just one:
const timerRef = React.useRef<{ start: number; end: number; remaining: number; id: NodeJS.Timeout | null }>({
    start: 0, // maybe we can use Date.now() as well instead of 0
    end: 0,
    id: null,
    remaining: duration,
  });
  1. In setTimeout we can use our recalculateRemainingTime prop for right delay before trigger:
setTimeout(() => {
  toast.onAutoClose?.(toast);
  deleteToast();
}, recalculateRemainingTime ? timerRef.current.remaining : duration);

Screenshots/Videos πŸŽ₯:

N/A

Copy link

vercel bot commented Mar 4, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
sonner ❌ Failed (Inspect) Mar 4, 2024 4:29am

@Flaain Flaain changed the title Flexible timer feat: added flexibility for timer Mar 15, 2024
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.

None yet

1 participant