ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

Radiobutton name attribute doesn't resolve correctly #3135

Open BartVanBeurden opened 6 years ago

BartVanBeurden commented 6 years ago

Description:

Radio buttons (input type = radio) are grouped together on the "name" attribute. However, expressions don't always translate into a "unique" name. This is especially true when passing the name to a component.

Versions affected:

0.9.9

Reproduction:

https://jsfiddle.net/3guuypgw/1/

{{#each radioGroupsBuggy as radioGroup:i }}
  <div>
    {{ radioGroup.name }} = {{ radioGroup.option }}
    {{#each radioOptions as option }}
      <RadioButton name="{{ radioGroup.option }}" value="{{ option }}" />
    {{/each}}
  </div>
{{/each}}
const RadioButton = Ractive.extend({
  template: '<input type="radio" name="{{ name }}" value="{{ value }}">'
 });

Expected behaviour: the <input> elements are rendered with name = {{ radioGroupsBuggy.0.option }}" instead of name = {{ name }} to avoid colliding name spaces.

evs-chris commented 6 years ago

This is an unfortunate side effect of the way name bindings co-opt the name attribute for something not directly related to the name attribute. The solution is to allow the name binding to also specify the value of the name attribute or perhaps to generate a unique name. I'm not entirely sure of the best approach for that, but two spring to mind:

  1. Allow a second interpolator to be specified in the binding for the actual name value e.g. <input name='{{.value}}{{@this._guid + 'group'}}' ... />
  2. Allow another attribute to specify the actual name if there's a name binding e.g. <input name={{.value}} name-value={{@this._guid + 'group'}} .../>

That would also resolve the issue of naming for people who submit their forms the old school way.

evs-chris commented 6 years ago

Are you using the radios in a form submission? If not, you could swap out the wrapping div for a form and the radios with the same name on different forms would be ungrouped e.g. https://jsfiddle.net/3guuypgw/2/.

BartVanBeurden commented 6 years ago

Interesting.

Not sure if this "resolves" the issue but it's probably the most optimal way to solve my problem.

evs-chris commented 6 years ago

It's a reasonable workaround. I'd say that the resolution would be to automatically include the instance guid in the generated name if the owning instance is a component while also putting in a hook to allow setting the name while using name bindings.

monoblaine commented 2 years ago

I had a similar issue. I had many radio buttons inside nested lists and I needed to have the name attribute be dynamically rendered so that form name-value pairs would be posted properly (and server-side form validation works properly). I also had to use radio input "name" binding so the "group" value of a radio is sent to the server and I needed two-way binding as well.

I'm posting my solution with the hope that someone gets benefit from it:

This solution assumes you always use strings as radio values.

Ractive.components.RadioGroup = Ractive.extend({
  template: '{{>content}}',
  isolated: false,
  data: () => ({
    name: '',
    values: [],
    selectedValue: '',
    required: false
  }),
  observe: {
    'values': {
      handler (newValues) {
        const selectedValue = this.get('selectedValue');
        if (selectedValue !== '' && !newValues.includes(selectedValue)) {
          this.set('selectedValue', '');
        }
      },
      strict: false,
      array: false,
      defer: true,
      init: false
    }
  }
});

Ractive.decorators.radio = function (node, name, value, required, selectedValueKeypath) {
  const context = this.getContext(node);
  node.setAttribute('name', name);
  node.setAttribute('value', value);
  if (required) node.setAttribute('required', '');
  node.checked = value === context.get(selectedValueKeypath);
  node.addEventListener('change', handleChange);
  const observer = context.observe(
    selectedValueKeypath,
    function (newValue) { node.checked = value === newValue; },
    {
      strict: true,
      array: false,
      defer: true,
      init: false
    }
  );

  return {
    teardown () {
      observer.cancel();
      node.removeAttribute('name');
      node.removeAttribute('value');
      node.removeAttribute('required');
      node.removeEventListener('change', handleChange);
    }
  };

  function handleChange (e) {
    context.set(selectedValueKeypath, e.target.value);
  }
};

const app = window.app = new Ractive({
  target: 'body',
  data: {
    myRadioGroupName: 'serhan',
    mySelectedValue: '2',
    myRadioOptions: [
      { value: '1', label: 'foo' },
      { value: '2', label: 'bar' },
      { value: '3', label: 'baz' },
    ]
  },
  computed: {
    myRadioValues () {
      return this.get('myRadioOptions').map(o => o.value);
    }
  },
  template: `
    <RadioGroup name="{{myRadioGroupName}}"
                values="{{myRadioValues}}"
                selectedValue="{{mySelectedValue}}"
                required="true">
      {{#each myRadioOptions as option}}
        <label>
          <input type="radio"
                 as-radio="name, option.value, required, 'mySelectedValue'">
          {{option.label}}
        </label>
      {{/each}}
    </RadioGroup>
  `
});

Ractive.js Playground link