svelte-headlessui icon indicating copy to clipboard operation
svelte-headlessui copied to clipboard

Use two-way binding

Open rgossiaux opened this issue 2 years ago • 4 comments

When writing this initially I imitated the React API as closely as I could, to make it easy to adapt the Tailwind UI snippets.

However, a second guiding principle is that I want this to feel like a native Svelte library as much as possible. I believe that Tailwind follows this approach themselves with their React/Vue versions. And I think a native Svelte library should really just use bind here. Most convincing, though, is that the Vue library uses data binding with v-model on several components.

Need to figure out the right way to semver this... I'm still on 1.0.0-beta.x but I think I might have to bump to 2.0 just for this change, not sure yet.

rgossiaux avatar Feb 02 '22 21:02 rgossiaux

I concur, building it the svelte is the way forward.

jeetrex avatar Feb 06 '22 14:02 jeetrex

Agreed! We are actively using this. I'd love to help out too. I understand the desire to align with upstream, but I think some concessions would be greatly appreciated. How can I help?

richarddavenport avatar Feb 07 '22 17:02 richarddavenport

Agreed! We are actively using this. I'd love to help out too. I understand the desire to align with upstream, but I think some concessions would be greatly appreciated. How can I help?

From a technical perspective it's a simple change. But since you ask... one thing that would be helpful if you or anyone else have a little time is converting some of the unit tests to using svelte-inline-compile, the way this test here does: https://github.com/rgossiaux/svelte-headlessui/blob/87360c0196be28e0dd8ea31f3a78e806ca7b5c3b/src/lib/components/listbox/listbox.test.ts#L123

Most of the tests were written before I discovered this library existed (it only had like 1 star on GH at the time), so I wrote my own component called TestRenderer to use my own custom syntax for testing multiple components together: https://github.com/rgossiaux/svelte-headlessui/blob/87360c0196be28e0dd8ea31f3a78e806ca7b5c3b/src/lib/components/listbox/listbox.test.ts#L156 The syntax of this is basically [Component, Props, Children]

However I'd like to convert these tests to using svelte-inline-compile eventually, because the syntax is much better, and it can handle some things that aren't possible using my TestRenderer. One of those is bind:. I think in order to test bind: properly, a test method will want to use a store and then bind to it in the component.

The components that will be affected by this issue are Listbox, RadioGroup, Switch, and Tabs, so the tests there that involve changing the values will be the ones that are highest priority to convert. But I'd welcome any PRs to convert any of the tests from the old TestRenderer, if anyone feels so inclined.

(edit: this is done now)

rgossiaux avatar Feb 09 '22 23:02 rgossiaux

It seems to me that this issue carries utmost importance. I keep going back and forth the documentation to remind myself how React implementation of a Svelte binding is written, namely on:change assign e.details to the value (??). And tbh, I use Vue versions of the snippets because of the import statements and all that, not React.

greendesertsnow avatar May 19 '22 05:05 greendesertsnow

Version 2.0.0 is now published with this change.

rgossiaux avatar Jun 11 '23 21:06 rgossiaux