radix-vue / shadcn-vue

Vue port of shadcn-ui
https://www.shadcn-vue.com/
MIT License
3.61k stars 207 forks source link

[Feature]: Make Switch work with v-model #121

Closed maelp closed 4 months ago

maelp commented 8 months ago

Describe the feature

It seems that right now we have to do something like this

              <Switch
                class="flex-none"
                id="toggle"
                :checked="isToggleSelected"
                @update:checked="(v) => (isToggleSelected = v)"
              ></Switch>

I guess it would be easier to allow something like

              <Switch
                class="flex-none"
                id="toggle"
                v-model="isToggleSelected"
              ></Switch>

but when I use the latter it doesn't update the var

Additional information

sadeghbarati commented 8 months ago

radix-vue intend to have the same API as radix-ui but I like v-model more cause you can easily use vee-validate componentField slotProp

radix-vue radix-ui
checked and v-model:checked checked
@update:checked onCheckedChange
romansp commented 8 months ago

v-model binding should work properly when used together with checked argument.

Try v-model:checked="isToggleSelected".

maelp commented 8 months ago

okay thanks! any reason that we don't simply use v-model since this seems to be the most natural?

sadeghbarati commented 8 months ago

okay thanks! any reason that we don't simply use v-model since this seems to be the most natural?

It's based on component design/props

https://vuejs.org/guide/components/v-model.html

flowchart LR
    C --> A["`2` checked (prop)"]
    A --> B
    B["`3` update:checked (emit)"] --> C
    C[`1` v-model:checked]
maelp commented 8 months ago

okay, not sure if I understood, but what I meant is: why using a "checked" variable rather than a "modelValue" variable, and using a simple v-model rather than v-model:checked?

RihanArfan commented 8 months ago

I believe usage for v-model:checked would be suited if a component had multiple things to change, such as something like <SignupForm v-model:username="" v-model:password="" />. However, since only has one thing, I agree with @maelp where only v-model should be used.

romanhrynevych commented 5 months ago

Yeah, I think that it must be ability to use v-model or v-model:checked (back compatibility). It is only Switch toggle, there will be no other v-model I think. But to update this we must change Radix-vue codebase, because all Props and Emits took from it, we just style components for shadcn styling 🙂

Think that need to add modelValue?: boolean; to SwitchRootProps, and 'update:modelValue': [payload: boolean]; to SwitchRootEmits) But I don't think that Radix-vue will accept this 🤔

image

@sadeghbarati maybe you can help here, maybe I don't familiar enough with codebase here ❤️

sadeghbarati commented 5 months ago

Radix Vue API intended to looks like main Radix UI Checkbox

https://github.com/radix-vue/shadcn-vue/issues/121#issuecomment-1763737779

Radix UI has checked prop and onCheckedChange function

Radix Vue has checked prop and update:checked emit which allow two way binding with v-model:checked

sadeghbarati commented 4 months ago

Pick the components you need. Use the CLI to automatically add the components, or copy and paste the code into your project and customize to your needs. The code is yours.

Feel free to change the source 😀

sugoidesune commented 2 weeks ago

I patched the emits for Checkbox. The original functionality is kept but v-model added. I think it's more than warranted to bring back expected vue functionality and dev experience even if shadcn-vue is just a "reskin" The main focus should be on VUE not REACT.js RADIX... https://github.com/radix-vue/shadcn-vue/issues/622#issuecomment-2178932075