skeletonlabs / skeleton

A complete design system and component solution, built on Tailwind.
https://skeleton.dev
MIT License
4.76k stars 299 forks source link

Support binding to RadioGroup value #54

Closed FeldrinH closed 1 year ago

FeldrinH commented 1 year ago

Currently the value for RadioGroup components has to be passed in and retrieved using a writable store.
It would be nice if you could bind to the value the same way you can with native input elements.

This would have two advantages:

  1. It would match the way you work with values when using native input elements
  2. It would be more convenient and flexible, especially when you want to bind to a value which is nested inside some larger data object.
FeldrinH commented 1 year ago

The current approach using a writable store feels especially inconsistent given that RangeSlider and SlideToggle do use binding.

thomasbjespersen commented 1 year ago

Thanks for bringing this up @FeldrinH.

@endigo9740 Was this a convention that was set early on? Variable dependency between parent and child components (being passed up->down)? In this case RadioGroup and RadioGroup item?

endigo9740 commented 1 year ago

It may help to understand the difference between the RadioGroup and RangeSlider/SlideToggle. That being that the RadioGroup is comprised of parent/child components like this:

<RadioGroup>
    <RadioItem />
    <RadioItem />
    <RadioItem />
</RadioGroup>

While the others are singular components like so:

<SlideToggle>...</SlideToggle>

This is important distinction. When you have the parent/child relationship between components, to my understanding, the best/only way to maintain state between them is the Context API.

With Context, data flows downward, through the parent, and into the children. However, it does not have a means to flow back up - that is unless you use something like a Store. So that's what we opted to do. Pass a store that represent the state, into the parent, which then is provided to the children. Then, given it's a store, it keeps state synced regardless of direction.

So it's not so much that we opted to skip the bind syntax by choice, in fact I always prefer something as close to native, but rather I'm not aware of a way do to what you're describing. At least not without flattening the parent/children components into a single component, which then destroys the descriptive nature of the template-driven components and comes with it's own pitfalls.

To my knowledge, bind cannot work with Context. However, if I'm wrong, or that's changed, then by all means I'd love to make the change suggested! The goal is always simplicity! It's been about 4 months since we setup this component and that data flow pattern, so it may be worth double checking what options are available.

I'll plan to take a look andl keep you posted @FeldrinH

endigo9740 commented 1 year ago

@FeldrinH I've had a chance to do some research and test this again today. However, there does not appear to be a means to use the bind syntax in combination with the Context API unfortunately. See my post above describing the data flow of the Context API to understand why (short version: data only flows down).

Additionally, I wanted to share this article which comes to the same conclusion: https://www.thisdot.co/blog/handling-forms-in-svelte

There's one caveat though, context is not reactive. But, we can make it reactive by making its content a store.

They are trying to maintain state for a reusable form, but the end result is the same. A store is really the only means to do this in Svelte at present.

Perhaps this use case would be a great showcase for a feature request to the Svelte team? Perhaps we can let them know that it would be great if is was possible for Context data to be reactive, either by default or with some kind of additional configuration per-instance? I'm sure they have some technical reasons for why it's not, but always nice to let them know when something isn't meeting expectations!

In the meantime the current syntax will remain as is. However I appreciate your suggestion all the same!

endigo9740 commented 1 year ago

@FeldrinH @thomasbjespersen I've gone ahead and created a feature request for a proposed reactive Context API. See the details here:

https://github.com/sveltejs/svelte/issues/7773

Feel free to chime in on there if you wish. The more visibility the better on these sorts of things.