remy / bind.js

bind.js - simple two way data binding to HTML and callbacks
694 stars 62 forks source link

Data binding on `select` element #23

Open remy opened 8 years ago

remy commented 8 years ago

A few issues have been raised about select elements not working, and it's definitely right, I started to look at the issue and it raises some questions.

It sort of makes sense that if the bound object is an array, each element in the array would be an option element. However, the point of changing the value of a select is to set the value of the select not particularly change the values.

This kind of goes against how bind.js works, since you might think the closest comparison is the ul element, however, when the array is changed against the ul, the contents of the ul changes entirely.

I feel like the callback to the bound object in the case of a select, should give the new set value.

Issues around this are:

  1. What if the returned value is not in the option list of the select element?
  2. Can you initialise the select from the bound object? If so, I can't see how without a feature change.
  3. Anything else?

For reference, this is what a ul data binding looks like:

<ul id="ul"></ul>

<script src="../lib/bind.js"></script>
<script>
var data = new Bind({
  cats: ['one', 'two', 'three'],
}, {
  cats: {
    dom: '#ul',
    transform: c => `<li>${c}</li>`
  },
});
</script>
greginns commented 8 years ago

For a <select> you need at least two things:

If you set an non-existent value it should just not select an <option>.

One really nice feature which I haven't seen in other libraries is support for the disabled attribute, which could be another value in the data array.

ShirtlessKirk commented 8 years ago

<select>s have intrinsic <optgroup>s as well, just for added fun...

DCSDB commented 8 years ago

I agree with greginns points, also I would like to bind to an array of objects with "key"/"value" properties. Showing the value and binding on the key. Would this be possible?

raminasri commented 8 years ago

Change this: "var valueSetters = ['SELECT', 'INPUT', 'PROGRESS', 'TEXTAREA'];" TO var valueSetters = ['OPTION', 'INPUT', 'PROGRESS', 'TEXTAREA'];

raminasri commented 8 years ago

Also change this: if (element.nodeName === 'INPUT' || element.nodeName === 'SELECT' || element.nodeName === 'TEXTAREA') { to if (element.nodeName === 'INPUT' || element.nodeName === 'OPTION' || element.nodeName === 'TEXTAREA') {

remy commented 8 years ago

The change isn't as simple as that. Sure, it fixes the options being set via an array, but then the value (in bind) is coersed from the array to a string.

These suggested changes half work (missing tests tho), but it raises the larger question: is bind.js okay with the value type magically changing?

raminasri commented 8 years ago

I agree with you. I still ahve problems after that. For exemple when I push a value to the array , the other value are changed also. (So the changes that has been made on those other value are lost.)

amuagarwal commented 8 years ago

Hi the issue with select element in lib file it working fine with https://cdn.rawgit.com/remy/bind/928c52ce335d6fd2c824b536a97efa460554b9e0/dist/bind.min.js seems something breaking in non-minified version

AwwDit commented 5 years ago

Just a heads up... There were two issues I had which I was able to fix: (bind.js not the minified version)

  1. My SELECTs defined in HTML were being wiped out by creating a bind to it. FIX: change var valueSetters = ['OPTION', 'INPUT', 'PROGRESS', 'TEXTAREA']; to var valueSetters = ['SELECT', 'INPUT', 'PROGRESS', 'TEXTAREA'];

  2. My binding to my SELECTs were not updating upon selecting a new value from the view. FIX: Line 324 this event is defined: var event = { //select: 'change', checkbox: 'change', radio: 'change' }[element.type];

This should be changed to

var event = { 'select-one': 'change', checkbox: 'change', radio: 'change' }[element.type];

The issue is that the type of a node SELECT is actually 'select-one'... if 'select-one' is not in quotes, the script will break due to the hyphen. Be sure to include the hyphen.

If you are manually changing the minified version the section I would be talking out is referenced as:

i={ checkbox:"change",radio:"change"}[n.type]

and should be changed to

i={'select-one': "change", checkbox:"change",radio:"change"}[n.type]

@remy