[bug]: Toasts don't fallback to DEFAULT_TIMEOUT

[brendanfalkowski]

Describe the bug

The useToasts hook appears to support a DEFAULT_TIMEOUT but the way it's implemented it can't be used.

You shouldn't need to specify the timeout every time a Toast is added.

To reproduce

Steps to reproduce the behavior:

1. Add a toast anywhere
2. But don't pass the timeout prop

Expected behavior

The Toast should timeout after the DEFAULT_TIMEOUT value specified in the hook.

What Actually Happens

Not passing timeout results in a TypeError. This means that every instance adding a Toast must specify the value which is bad for consistency.

Possible solutions

I will PR a fix shortly.

Please let us know what packages this bug is in regards to:

• venia-concept
• venia-ui
• pwa-buildpack
• peregrine
• pwa-devdocs
• upward-js
• upward-spec
• create-pwa

[m2-assistant]

Hi @brendanfalkowski. Thank you for your report.
To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[brendanfalkowski]

@magento I am working on this

[sirugh]

You shouldn't need to specify the timeout every time a Toast is added.

Yea, the docs indicate the prop is optional. And the code reads like it should function without it. I'm curious what the error is.

Edit:

I threw a toast into useApp and did not specify timeout and got an error that seemed pretty self explanatory.

TypeError: Toast should be user-dismissable or have a timeout

Adding dismissable: true (and no timeout) to the addToast args stopped the error from throwing and dismissed the toast after the default timeout.

[brendanfalkowski]

@sirugh Hmm, that seems to just trade timeout for a different prop dismissable that needs to be specified. The normal behavior for toasts (in wider use) seems to be they default to self-dismissing. Just short confirmations that don't really require attention or action, unless you override it.

Ideally, the mental footprint to use it should be low so it's just { message, type } being passed in most cases. It still seems weird to have to specify dismissable: true when there should be a default timeout.

We ended up modifying this to fit the behavior of this pattern in our M1 site to support a title prop as well, and just applying the DEFAULT_TIMEOUT as a param fallback seemed like the cleanest fix for how we use it. But I might be misunderstanding the intent.

[sirugh]

But I might be misunderstanding the intent.

@brendanfalkowski maybe :D I wrote this stuff almost two years ago and I've slept very little since then so we're in the same boat. At some point it might be worth it for us to clean up the toast implementation. For now, I think we can just accept your PR as is.

Also, sorry about the spam on this issue - we're trying to figure something out with our jira-github sync tool.

[github-jira-sync-bot]

✅ Jira ticket "PWA-1382" is successfully created for this issue.

[brendanfalkowski]

@sirugh I hear you. I wrote myself the // TODO: why do we have to specify timeout in every call? comment 7 months ago, and just ran across it again last week, so debugged. Eventually, it gets better.