skeleton icon indicating copy to clipboard operation
skeleton copied to clipboard

Toast timeout closing issue

Open MeroVinggen opened this issue 1 year ago • 7 comments

Current Behavior

<Toast/> triggered with the timeout property will close the new toast, if it was opened at the same time as the current one is closing.

Expected Behavior

<Toast/> triggered with the timeout property should not close the newly opened one.

Steps To Reproduce

You may reproduce this behavior by any toastStore.trigger call with a short gap between prev timeout and the new toast show trigger.

Link to Reproduction / Stackblitz

https://svelte.dev/repl/4e7d508de8c54546afa090755b677563?version=4.2.8a

More Information

No response

MeroVinggen avatar Jan 11 '24 11:01 MeroVinggen

@MeroVinggen unfortunately your REPL isn't working for me. It remains stuck here:

Screenshot 2024-01-11 at 11 20 30 AM

Would you mind perhaps setting up a Stackblitz for us? We have a version with Skeleton setup, which is linked on the homepage of our doc site:

https://stackblitz.com/edit/skeletonlabs-repl-znwovn?file=README.md

Otherwise we can try to replicate locally based on the code provided.

endigo9740 avatar Jan 11 '24 17:01 endigo9740

@endigo9740 here you go:

https://stackblitz.com/edit/skeletonlabs-repl-hxhxxd?file=src%2Froutes%2F%2Bpage.svelte

Short REPL description:

  • if you press "open toast" it will call toastStore.trigger
  • if you check the "open new toast at the same time as prev will close (1s timeout)" checkbox it will call toastStore.trigger on a 1s timeout after you click "open toast" to show you the issue

MeroVinggen avatar Jan 12 '24 16:01 MeroVinggen

Thank you @MeroVinggen. I've actually seen this behavior but had a hard time replicating it, so this will be super helpful. Much appreciated!

endigo9740 avatar Jan 12 '24 16:01 endigo9740

@MeroVinggen In this unique scenario, the bug does not originate from the toast code; rather, it arises due to the sequence of timers. When the execution time of the open() function exceeds the delay specified in setTimeout(), the second toast appears prematurely, leading to the observed occurrence where the second toast closes simultaneously with the first one.

to solve this issue you can simply swap the order in your function.

  const open = () => {
    if (openNewToast) {
      setTimeout(open, 1000)
      openNewToast = false;
    }

    toastStore.trigger({
      message: "Lorem ipsum...",
      timeout: 1000,
    });
  };

Mahmoud-zino avatar Jan 13 '24 11:01 Mahmoud-zino

I'd be curious to hear of Mero has seen this happen in normal use though. If so, that may be something we monitor going forward.

endigo9740 avatar Jan 13 '24 18:01 endigo9740

@endigo9740 Glad you asked, yeah, I faced this issue in plain usage. You need to get in timing when the previous toast is about to close, right before this if you call a new one - both will be closed.

The stackblitz example was just an idea of how to reproduce it in ez way. It's a bit tricky but you can try it in my stackblitz, click to call new toast, and click again in about a second and you will face the issue.

MeroVinggen avatar Jan 13 '24 20:01 MeroVinggen

hmm this is interesting, I will have to dig deeper...


Edit: ok I can now confirm deleting the transitions is solving the issue, so the bug should be located somewhere in the transitions logic.

Mahmoud-zino avatar Jan 14 '24 19:01 Mahmoud-zino

I found this bug in the wild and it only seems to happen whenever transitions are enabled and a second toast is triggered immediately after the first one is closed.

Weirdly if we have a queue up 3 toasts in a way that the first one is visible and the third one is triggered immediately after the second is closed this behavior does not happen.

idk how or why this happens, anyhow delaying the closing of the first toast to be AFTER the trigger of the second toast seems to fix this, but this is a bandaid fix at best

VitAndrGuid avatar Apr 15 '24 23:04 VitAndrGuid

Vit volunteered to take this one on Discord, so it's now assigned!

endigo9740 avatar Apr 16 '24 00:04 endigo9740

@endigo9740 This seems to be related to svelte edgecases regarding transitions and conditional blocks, and not a issue with the toast store, ive created a REPL based on the Toast.svelte component and the toast store to narrow the bug.

https://svelte.dev/repl/a4a4596cf8b341e9b2c0532a1ec03e71?version=4.2.14

VitAndrGuid avatar Apr 16 '24 22:04 VitAndrGuid