Feat: toast enhancement

[Mahmoud-zino]

Linked Issue

Closes #1311

Description

Done

1. ability to hide the x button using the prop hideDismiss.
2. ability to remain visible if hovering using the prop hoverVisible.
3. ability to close toast programmatically using the toastId.

Notes

1. I was worried that the user could activate hideDismiss and the toast will stay on screen infinitely. so I have set autohide to true automatically when hideDismiss is set and wrote a note in the docs about it.
2. Should I add some hover effect when hoverVisible is set so the user knows that hovering the toast will keep it open?
3. remaining visible when hovering is simpler when we just delete the timeout and recreate it after, but the time spent on the toast is not calculated, so should I track the time and kind of continue it after mouseLeave or is it ok to just reset the timer?
4. to close the toast programmatically I am returning the ID from the store trigger function, I don't think this change should be a breaking change, but I just wanted to make sure.

Changsets

feat: Toast enhancement - ability to hide the close button, ability to remain visible on hover, ability to close toasts programmatically by Id.

Checklist

Please read and apply all contribution requirements.

• This PR targets the dev branch (NEVER master)
• Documentation reflects all relevant changes
• Branch is prefixed with: docs/, feat/, chore/, bugfix/
• Ensure Svelte and Typescript linting is current - run pnpm check
• Ensure Prettier linting is current - run pnpm format
• All test cases are passing - run pnpm test
• Includes a changeset (if relevant; see above)

[changeset-bot]

🦋 Changeset detected

Latest commit: 88dd8cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@skeletonlabs/skeleton	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
skeleton-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 14, 2023 4:59pm

[saturnonearth]

wow you literally went through every change in my list and knocked it out of the park!

ability to close toast programmatically using the toastId.

If someone wanted to use this, Is there a way to grab the ID from somewhere other than the callback?

Should I add some hover effect when hoverVisible is set so the user knows that hovering the toast will keep it open?

I don't think it's that needed but maybe a slight transform to make it slightly bigger on hover?

remaining visible when hovering is simpler when we just delete the timeout and recreate it after, but the time spent on the toast is not calculated, so should I track the time and kind of continue it after mouseLeave or is it ok to just reset the timer?

I think resetting it is fine.

[endigo9740]

@Mahmoud-zino I'll circle back to this soon - but make sure you adjust the Changeset to be a paragraph style. This is the data that'll appear on the generated changelog, so if it's an odd format it may not come through as desired. We should have our first look at the changelog format in the next hour or so, so keep an eye out for that.

[Mahmoud-zino]

@saturnonearth

If someone wanted to use this, Is there a way to grab the ID from somewhere other than the callback?

Is there a specific use case you could use? I thought it only makes sense to close them when you trigger them -> you grab the Id from the callback, but if there is another use case I would be happy to implement it.

[saturnonearth]

@saturnonearth

If someone wanted to use this, Is there a way to grab the ID from somewhere other than the callback?

Is there a specific use case you could use? I thought it only makes sense to close them when you trigger them -> you grab the Id from the callback, but if there is another use case I would be happy to implement it.

I guess I just wondered if you could set a custom ID yourself instead of having to grab it from the callback.

[Mahmoud-zino]

I guess I just wondered if you could set a custom ID yourself instead of having to grab it from the callback.

I understand where this is coming from, but the user can't do anything with the Id other than close the toast, and if it is not random/unique generated it could lead to some unwanted behaviors. so I think it is better to handle it ourselves...

If in the future we had any situation where the user could use the ID we can export it very easily.

[endigo9740]

Hey @Mahmoud-zino, the logic for these new features looks sound. The only issues I have are in relation to param naming and the documentation presentation. If you can implement the following I'd be happy to merge this:

Param changes:

• Change hideDismiss: true -> dismissable: false (with true being the default)
• Change hoverVisible -> hoverable: true (with false being the default)

Doc Changes:

• Change the button text for the hover option to "Visible on Hover" (with caps as shown)
• For the hover example -> code tab, let's reword the warning as follows:

When using the dismissable option autohide is required and enabled by default.

• Let's split the Programatic example to it's own heading section directly after Trigger/Clear called "Programatic", add the text I supply below for the description, then show your exaple with buttons labeled "Show" and "Close". The idea being that we might add additional programatic features in the future.

Create a reference ID for your toast so that you may programatically close the toast on demand.

Additionally you may want to modify the changeset to more accurate represent the changes above. Remember the changeset should only describe the change, not provide the implementation details. That's where the PR link and documention comes into play. I'd recommend something like:

feat: Expanded Toast features to allow hiding the dismiss button, allow toast to remain visible on hover, and programatically close each instance

[Mahmoud-zino]

@endigo9740

Change hideDismiss: true -> dismissable: false (with true being the default)

from what I know, interfaces can't have a default value, do you have another suggestion for the name (opposite of dismissable, I was thinking undismissable but it sounds a little bit odd)?

[endigo9740]

@Mahmoud-zino

from what I know, interfaces can't have a default value, do you have another suggestion for the name (opposite of dismissable, I was thinking undismissable but it sounds a little bit odd)?

Sorry I meant the default behavior - not anything that has to be defined in the code. But I do agree we should flip the semantics meaning the other way around. Let me check what other libraries do for this.

[Mahmoud-zino]

@endigo9740
I applied the changes you suggested and changed the hideDismiss to undismissable for now, but I will be happy to change it once you have a better idea (I searched a lot with no luck).
Also, you might want to look at this comment regarding the node feature: #1311 (comment)

[endigo9740]

@Mahmoud-zino unfortunately undismissable is not really a standard word, so we really can't use that. I'd suggest either flipping back to hideDismiss: true or dismiss: false. I did find find one reference using the latter:

https://github.com/mzohaibqc/svelte-toasts

(see the readme)

I agree flipping between positive/negative boolean values feels weird, but as long as it's documented it should work. The other option would be to make dismiss a off-by-default feature, then having dismiss: true be the optional setting. But that would be a breaking change, so not too keen to have that right now. We might consider it for v2 though

[Mahmoud-zino]

@endigo9740 The problem is we can't change the default value of the prop (because we are using an interface) so I don't understand what you meant by flipping back to "hideDismiss: true" (also in PR it was false) if we could somehow have true as default value I would make sure to document it but I don't know if it is possible.

[endigo9740]

@Mahmoud-zino again the default state is implicit rather than explicit within the code. If a value like hideDismiss is not
provided then the toast will behave in it's "default" manner.

• Default: dismiss button is present
• hideDismiss: true - dismiss button is no longer provided

This will be our short term implementation for this feature. But when v2 rolls around we might want to swap the defaults to be like this:

• Default: dismiss button is not present
• dismiss: true - dismiss button is provided

However, this would be a breaking change, so we cannot implement this right now. But the semantics and state of dismiss: true is more inline with expectation for configurable options.

Passing false is not the common paradigm here. But we're limited to what we can do within the constrains of how this operates right now, we cannot implement a breaking change.

[Mahmoud-zino]

@endigo9740 aww now I get you, yeah sorry for misunderstanding that like 3 times (Monday effect 😶)

It makes sense, I will make the change 👍.

[endigo9740]

@Mahmoud-zino I fixed a small style issue that was providing a gap on the right side of the toast when the dismiss option is hidden. Likewise the docs felt like a ton of examples without context, so I've adjusted this to provide more information about each type of setting. I think with this we're good to merge!