Feature: Added API for using slots for modal content and closebutton

[oddsund]

Background

I use svelte-simple-modal in a budgeting app. The modal is used to display suggestions for similar transactions when a user performs some edit on one transaction, like "You deleted this transaction, would you also like to delete these similar transactions?".

Currently Observed Behavior

As I want the user to be able to just update part of the list, current behavior would require me to store and update similar transactions in a store. However, I would like to pass it in as an argument, to make the binding of data explicit at the call site.

Further, I would like to colocate the definition of the modal content and the modal like regular svelte code, as I feel it makes it a bit more clear what is the content of the modal.

New Behavior

This PR adds an API for using slots with this library. If no slots are provided, it simply uses the current functionality.
It also adds a new argument to the Modal Component, callbacks, which can be used both with the current functionality and the new slots functionality.

I do think that the new callbacks prop can also have its uses for the current functionality, as one can then add callbacks that are tied to the modal, and not just the content.

Demo: (https://svelte.dev/repl/136d296195b543c4a4a6de127ff9e06e?version=3.35.0)
It's an updated version of the main demo of svelte-simple-modal

[oddsund]

I see now that I forgot to mention and commit the update of Svelte version.

To support named slots, minimum Svelte version has to be bumped to 3.35.0. I'll update the package.json and lock once I'm on my laptop later today.

[oddsund]

I see now that I forgot to mention and commit the update of Svelte version.

To support named slots, minimum Svelte version has to be bumped to 3.35.0. I'll update the package.json and lock once I'm on my laptop later today.

Fixed in a60cea8

[flekschas]

Thank you for putting together this PR. I'll take a look in the next few days.

[flekschas]

Thanks again for putting the PR together and sorry it took me a bit to look at it I have a few questions about the purpose of the proposed changes.

The slot approach

As I want the user to be able to just update part of the list, current behavior would require me to store and update similar transactions in a store. However, I would like to pass it in as an argument, to make the binding of data explicit at the call site.

Do you mind provide a little code example demonstrating how the current API makes your use case tricky and how the slot-approach simplifies this? This might just be me, but I have a hard time visualizing where the current bottleneck is.

Also, if I understand correctly, the slot-based approach would be useful for cases where you know ahead of time which modal should be shown. Or how would it be possible to show different content using the same <Modal /> instance? E.g., in the original example, there's only one <Modal /> instance needed for displaying three different types of modal content components. Am I understanding correctly that the slot approach would require the user to wrap each content component in a separate <Modal /> instance?

From my current understanding that does not seem very efficient as the modal logic is always the same and shouldn't need to be repeated. But again, maybe I am not fully understanding the use case.

The callbacks property

What does this new property allow you to do that that you can't already achieve with on:open, on:opened, on:close, on:closed or the open() and close() functions?

My comments might seem a bit critical but I am just trying to understand how adding those changes help simplifying things because for both, the content and callbacks, the <Modal /> already provides two ways of achieving the same thing. Adding a third way might be possible, but it'll impact the maintenance burden and might confuse new people who might ask themselves, which approach they should use.

[oddsund]

Thanks again for putting the PR together and sorry it took me a bit to look at it I have a few questions about the purpose of the proposed changes.

No worries! Sorry that my reply took time as well, just finished paternal leave so energy for off-hours coding has been a bit limited.

The slot approach

As I want the user to be able to just update part of the list, current behavior would require me to store and update similar transactions in a store. However, I would like to pass it in as an argument, to make the binding of data explicit at the call site.

Do you mind provide a little code example demonstrating how the current API makes your use case tricky and how the slot-approach simplifies this? This might just be me, but I have a hard time visualizing where the current bottleneck is.

A simple example is her : https://svelte.dev/repl/7fbd80e76c114abab1565cb2eb4d9808?version=3.39.0
Note that I bumped Svelte version to 3.39.0 to remove a false warning(sveltejs/svelte#6065)

So the problem is that the current API doesn't enable reactive prop passing and passing of content as a regular component. This means that you have to use a store even for simple content with only one prop, if you want it to be reactive.
So while it's not tricky in the current API, it's somewhat cumbersome, as all use cases have to be solved by using stores or context.
Slots, on the other hand, enables one to implement this using regular props. Even bind:prop works as usual.

Also, if I understand correctly, the slot-based approach would be useful for cases where you know ahead of time which modal should be shown. Or how would it be possible to show different content using the same <Modal /> instance? E.g., in the original example, there's only one <Modal /> instance needed for displaying three different types of modal content components. Am I understanding correctly that the slot approach would require the user to wrap each content component in a separate <Modal /> instance?

All of those are correct. One modal would be used for one content component, unless you wrap it in e.g. a fragment and switch content using if's.
I guess you could swich content component type by setting the component in a reactive statement, but I can't think of a use case for that at the moment.

From my current understanding that does not seem very efficient as the modal logic is always the same and shouldn't need to be repeated. But again, maybe I am not fully understanding the use case.

I guess you could say that the slot API is a tradeoff between efficiency(possibly duplicating Modal logic) and usability(content to the modal is passed as normal, and showing it is done with a prop). Also, I think it would be a bit clearer for the reader what the content of the modal would be, as the content is set in one place, and one place only - the callsite.

The callbacks property

What does this new property allow you to do that that you can't already achieve with on:open, on:opened, on:close, on:closed or the open() and close() functions?

You're absolutely right, and I have no good explanation for why I didn't think of that. I'll remove the callbacks property if the slot-approach is accepted for merge.