-
Notifications
You must be signed in to change notification settings - Fork 8
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
v2.0.0 #22
base: main
Are you sure you want to change the base?
v2.0.0 #22
Conversation
openModal is now a promise that resolves with the value passed in by closeModal
Event handling
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@MentalGear @c00 this is pretty much ready to go, but I'd love if you could try it and share any feedback you might have as it largely focuses both of your raised issues. Otherwise, this will probably get released sometime next week! It's available as |
This is awesome, thank you - I'll try to check it out in the next days!
… On 20 Aug 2022, at 23:55, Matt Jennings ***@***.***> wrote:
@MentalGear <https://github.com/MentalGear> @c00 <https://github.com/c00> this is pretty much ready to go, but I'd love if you could try it and share any feedback you might have as it largely focuses both of your raised issues. Otherwise, this will probably get released sometime next week!
It's available as ***@***.*** and the latest docs can be seen here <x-msg://4/svelte-modals.next.mattjennin.gs>.
—
Reply to this email directly, view it on GitHub <#22 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVUVGYREHVVTYJGUOURSUTV2FH5JANCNFSM57D2TDSA>.
You are receiving this because you were mentioned.
|
I like this a lot! I'm super happy with the way we await results. The event dispatching also seems well done! One of the things that I found a bit confusing at first is that it's not obvious that the |
Second the notion on modal props. v.2 would be a great opportunity to hide them in context! |
Much appreciate the new "createModalEventDispatcher", which usage is also much more idiomatic for svelte instead of the previous callback. This could become the definitive svelte modals lib! |
@c00 Re: props I share the same concern. I did give a context-like approach some thought initially but felt it would be too boilerplate-y. But I am willing to give it more thought. This was roughly my idea: <script>
import { register } from 'svelte-modals'
const $modal = register({
// probably would be renamed to preventClose or something
onBeforeClose: () => {},
/* ... other eventual config */
})
// would provide
$modal.isOpen
$modal.close()
$modal.dispatch()
</script>} alternatively <script>
import { register } from 'svelte-modals'
const { isOpen, close, dispatch }= register({ ... })
// potential foot-gun. if someone didn't know isOpen was a store, it would always be truthy
$isOpen
// but you wouldn't need to access everything else through $modal
close()
dispatch()
</script>} What would you think of either approach? This actually would not be using context, hence why @MentalGear I'll take a look at the backdrop fading, thanks! I'm glad you are both happy with the events/returned value solutions though! |
I am unfamiliar with the
This approach is my favorite. It also seems to be most svelte-ish. Assuming it's typed it correctly, the IDE will know what the properties are, so it shouldn't be too much of a problem. (Also, every example of a modal on the svelte REPL uses a store for its |
Apologies for the lack of activity on this. It's been a busy end of the year for myself and I've had very little time or energy to devote to OSS in general. I can't make any promises on timelines but I want to give this API some serious thought before committing to it. At this point, I think it's most likely to be this: <script>
import { register } from 'svelte-modals'
const { isOpen, close, dispatch }= register({ ... })
</script> |
That approach works well for me.
I hope you're doing well, man. Have a great end of the year 🎉 |
Looking forward to get this released. 👍 |
No description provided.