sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.21k stars 4.09k forks source link

`bind:group` does not work with nested components #2308

Open imbolc opened 5 years ago

imbolc commented 5 years ago

I'm trying to bind a store variable to group of checkboxes and it works till I move checkbox into a separate component, after that only one checkbox can be chosen at a time, here's an example from repl: https://gist.github.com/imbolc/e29205d6901d135c8c1bd8c3eec26d67

Rich-Harris commented 5 years ago

Yeah, this won't work as things currently stand — the group has to be within the component.

I suppose we could have a global Map of binding groups, though I'm not sure what the change notification looks like (for components other than the one with the element from which a change originated). This could be something to ponder post-3.0.

bestguy commented 4 years ago

Is there a workaround for this in v3? It makes components that use a checkbox pretty difficult/confusing to use if you need to use groups.

https://svelte.dev/repl/1565708677134e418e256234984d90ef?version=3.12.1

NikolayMakhonin commented 4 years ago

Solution of this problem but without keep order of the selected items:

https://svelte.dev/repl/de117399559f4e7e9e14e2fc9ab243cc?version=3.12.1

NikolayMakhonin commented 4 years ago

Universal toggle component:

https://svelte.dev/repl/b63c813830274248a8fee5ecc667b15c?version=3.12.1

boleeluchshiy commented 4 years ago

I know, this is obvious, but still. For those, who run into the same problem: It would be sufficient to make separate component for the group of checkboxes, e.g. CheckboxGroup, which you can bind to a group <CheckboxGroup { checkboxes } bind:group/>.

See the example and learn some russian words :) https://svelte.dev/repl/faabda4cabd544bd858a8a8abd0095f5?version=3.12.1

kevmodrome commented 4 years ago

This came up in the Discord today, what would be needed to make this work? Intuitively I feel like this should work out of the box.

tamasPetki commented 4 years ago

What if you try like this?:

https://svelte.dev/repl/42ff2937ebe345cc997fd080c347567c?version=3.20.1

didier commented 3 years ago

Is this still being looked at? Would love for this to "just work", much like the rest of Svelte 😄. If there's anything I can do to help, let me know.

arggh commented 3 years ago

What if you try like this?:

@tamasPetki if you switch the inputs' type to "checkbox", you can only select one item in your example.

https://svelte.dev/repl/45496f841fef41cc91012b12abf3f3fa?version=3.20.1

SystemDZ commented 3 years ago

You will love this https://svelte.dev/repl/6a86a48ae33e4412935016c606136377?version=3.37.0

didier commented 3 years ago

@SystemDZ Your example doesn't use nested components. The issue only occurs when nesting a component and trying to bind them to an input group.

autumnloveless commented 3 years ago

is there any update to this? Trying to figure this out myself.

Masstronaut commented 3 years ago

Just encountered this issue. I'm fine with the workarounds above, but I'm less thrilled with spending several hours figuring out I didn't have a bug all along - it was in svelte. I'm sure plenty of other people will also spend several hours banging their heads against the wall trying to figure out what they are doing wrong.

7antra commented 3 years ago

Absurd & hacky but works : https://svelte.dev/repl/02d60142a1cc470bb43e0cfddaba4af1?version=3.38.3

EDIT : @locriacyber found a better solution here : https://github.com/sveltejs/svelte/issues/2308#issuecomment-1005942571

didier commented 3 years ago

Absurd & hacky but works : https://svelte.dev/repl/02d60142a1cc470bb43e0cfddaba4af1?version=3.38.3

@7antra Hahaha, that's beautiful. Thanks for sharing.

silvestrevivo commented 3 years ago

@7antra you saved my day!!! thanks a lot!!!

Abibibi commented 2 years ago

@7antra Yann!! Thank you so much for your help, coworker ;)

joshuawjulian commented 2 years ago

Is there any movement forward with this? I spent hours to figure out nested binds are the problem and not me.

iacore commented 2 years ago

Here's a simple solution without bind:group: https://svelte.dev/repl/6a17f4105e2b4bcd8d2df3eaff5bce0d?version=3.38.3

~~Here's a more complicated solution by passing one store to each checkbox component: https://svelte.dev/repl/c8f4a50b563a4ebd9c9286aba98d4806?version=3.38.3~~

It is impossible without cross-component static analysis. It is possible only when the input (checkbox) only set or unset its own element, without deleting existing choices in a list. For example, unchecking a checkbox with value "foo" will only remove "foo" from the bind:group array, without touching other components. This is the solution in https://github.com/sveltejs/svelte/issues/2308#issuecomment-539954299, and I think it should be the default semantic of bind:group for checkboxes.

Demo: https://svelte.dev/repl/eeed24ad43cf4b01a53b75c5889afca6?version=3.44.3

H40831 commented 5 months ago

Isn't it possible to resolve this by reimplementing bind:group?

<script>
  let checked = false;
  export let value = '';
  export let group = [];

  const handleCheck = () => { group = [...group, value] };
  const handleUncheck = () => { group = group.filter(checkedValue => checkedValue !== value) };

  $: checked? handleCheck(): handleUncheck();
</script>

<input type="checkbox" value={value} bind:checked>

and use it in the parent component

<script>
  let group = [];
</script>

<Checkbox value="xxx" bind:group={group} />
webJose commented 5 months ago

Stopped by to support the idea of fixing this. Thanks!

My Scenario

A component library for a company-tailored Bootstrap. Would like to create a Checkbox component that handles all the Bootstrap things, and would like to provide the group property as part of the component. As it is right now, checkboxes behave like radio buttons.

bestguy commented 5 months ago

@webJose yes, this is what we ran into with Sveltestrap's Checkbox component for bootstrap when commented above.

webJose commented 5 months ago

@bestguy the good thing, I suppose and thanks to @H40831, the behavior can be emulated pretty decently.

katriellucas commented 4 months ago

I have found myself with the same problem, each solution seems to have some sort of downside, for example:

For now, using a "checked" prop seems less verbose while having none of the above downsides, but it doesn't respect the insert order either:

This is a Svelte 5 solution, but you can make it on Svelte 4 as well:

<!-- app.svelte -->

<script>
  import Checkbox from './Checkbox.svelte';

  let options = $state([
    { "value": "t1", "checked": false },
    { "value": "t2", "checked": false },
    { "value": "t3", "checked": false }
  ])

  let der = $derived(options.filter(o => o.checked === true).map(o => o.value))
</script>

<button onclick={() => options.unshift({ "value": "t"+options.length, "checked": false })}>add</button>
<button onclick={() => options = options.reverse()}>reverse</button>

<br><br>

{#each options as opt }
  <Checkbox value={opt.value} bind:checked={opt.checked}/>
{/each}

<br><br>

Checked {der}

<!-- Checkbox.svelte -->

<script>
  let { checked = $bindable(), value } = $props()
</script>

<label>
  {value}:<input type="checkbox" {value} bind:checked />
</label>
iacore commented 4 months ago

@imbolc I fixed your example in #11256.

webJose commented 4 months ago

@iacore I don't think this is what is needed. What we want is to be able to bind:group over t he nested component.

Instead of this:

{#each menu as flavour}
    <Flavour {flavour}/>
{/each}

<script>
    import Flavour from "./Flavour.svelte"
    let menu = [
        'Cookies and cream',
        'Mint choc chip',
        'Raspberry ripple'
    ];
</script>

We want this (note the binding in the Flavour component):

{#each menu as flavour}
    <Flavour {flavour} bind:group={selection} />
{/each}

<script>
    import Flavour from "./Flavour.svelte"
    let menu = [
        'Cookies and cream',
        'Mint choc chip',
        'Raspberry ripple'
    ];
        let selection = [];
</script>
iacore commented 4 months ago

We want this (note the binding in the Flavour component):

{#each menu as flavour}
  <Flavour {flavour} bind:group={selection} />
{/each}

<script>
  import Flavour from "./Flavour.svelte"
  let menu = [
      'Cookies and cream',
      'Mint choc chip',
      'Raspberry ripple'
  ];
        let selection = [];
</script>

This is fixed as well in https://github.com/sveltejs/svelte/pull/11256.

See PoC: https://github.com/iacore/fix-svelte-bind-group

https://github.com/iacore/fix-svelte-bind-group/blob/main/src/App.svelte

didier commented 3 weeks ago

Is this being worked on? It's not immediately clear when following the issues and PRs.

dummdidumm commented 3 weeks ago

It is not being worked on. The big problem is how to get a reference to the underlying variable such that it can be updated and not just read

didier commented 3 weeks ago

What if group were a prop? Why is it hard to get a reference?

blujedis commented 2 weeks ago

For posterity sake here's what I came up with and example of the issue...

If my lib I'm detecting what the user has passed in to "group" and then using bind:group for single values or using onchange event with $state() in the parent to get updated changes.

This way the user always uses bind:group

https://www.sveltelab.dev/fb2v6udij7g5jvp

7nik commented 2 weeks ago

Yeah, in Svelte 5, it seems to be easy to overcome. And re-creating the array instead of mutating gives better result: REPL.

But there is one big difference - bind:group preserved the order of values, and it's the trickiest behaviour to replicate. By the way, Vue doesn't do so, and switching between single and multiselection is done by checking whether the supplied value is an array. But I don't like this fuzziness.

Here is my attempt to preserve the order - not a trivial task, but not that big in the end. One potential issue I see in this approach is that if two groups of inputs will be initiated with the same group they are detected as one group.

iacore commented 2 weeks ago

Is this being worked on? It's not immediately clear when following the issues and PRs.

I fixed it. @Rich-Harris says no, so it's not getting fixed.