value binding on select doesn't respond to null when not using options binding #493

Merged
merged 2 commits into from Mar 17, 2013

Conversation

Projects
None yet
4 participants
@mbest
Member

mbest commented Mar 16, 2013

Discovered when trying to work around #472 if you don't use the options binding, it doesn't appear that knockout will drive the value binding when using null.

Example case: http://jsfiddle.net/SfpVd/1/

Steps

  1. go to the fiddle above
  2. change the selection to 3
  3. click the button
  4. see how it does not reset
@joeldart

This comment has been minimized.

Show comment Hide comment
@joeldart

joeldart May 17, 2012

Contributor

A couple notes on this behavior: you can't set the option value to null, it you omit the value property it works in knockout but according to spec http://www.w3.org/TR/html4/interact/forms.html#edef-OPTION the initial value would then be "Select a Value" so your fallback wouldn't be valid.

As a workaround, setting the observable to "" instead of null works for my use case, but this acts differently from if you'd bound to the options binding and optionText. I don't know if there's a solution to this as I found setting to "" in selectExtensions if the value is null and not found explicitly in the list causes a regression in ie9 ("For select boxes, should automatically initialize the model property to match the first option value if no option value matches the current model property value").

Contributor

joeldart commented May 17, 2012

A couple notes on this behavior: you can't set the option value to null, it you omit the value property it works in knockout but according to spec http://www.w3.org/TR/html4/interact/forms.html#edef-OPTION the initial value would then be "Select a Value" so your fallback wouldn't be valid.

As a workaround, setting the observable to "" instead of null works for my use case, but this acts differently from if you'd bound to the options binding and optionText. I don't know if there's a solution to this as I found setting to "" in selectExtensions if the value is null and not found explicitly in the list causes a regression in ie9 ("For select boxes, should automatically initialize the model property to match the first option value if no option value matches the current model property value").

@mbest

This comment has been minimized.

Show comment Hide comment
@mbest

mbest Jun 7, 2012

Member

I'd like to fix this, but not sure if we'll get to it right away. I believe changing the code in selectExtensions.writeValue for select to this could fix it for you:

                case 'select':
                    if (value == "")
                        value = undefined;
                    if (value == null)
                        element.selectedIndex = -1;
                    for (var i = element.options.length - 1; i >= 0; i--) {
                        if (ko.selectExtensions.readValue(element.options[i]) == value) {
                            element.selectedIndex = i;
                            break;
                        }
                    }
                    // for drop-down select, ensure first is selected
                    if (!(element.size > 1) && element.selectedIndex === -1) {
                        element.selectedIndex = 0;
                    }
                    break;
Member

mbest commented Jun 7, 2012

I'd like to fix this, but not sure if we'll get to it right away. I believe changing the code in selectExtensions.writeValue for select to this could fix it for you:

                case 'select':
                    if (value == "")
                        value = undefined;
                    if (value == null)
                        element.selectedIndex = -1;
                    for (var i = element.options.length - 1; i >= 0; i--) {
                        if (ko.selectExtensions.readValue(element.options[i]) == value) {
                            element.selectedIndex = i;
                            break;
                        }
                    }
                    // for drop-down select, ensure first is selected
                    if (!(element.size > 1) && element.selectedIndex === -1) {
                        element.selectedIndex = 0;
                    }
                    break;
@CloCkWeRX

This comment has been minimized.

Show comment Hide comment
@CloCkWeRX

CloCkWeRX Nov 8, 2012

I find I run into this frequently, whenever mixing rails generated HTML with knockout controlled forms. The confusion/frustration factor is unfortunately quite high.

I find I run into this frequently, whenever mixing rails generated HTML with knockout controlled forms. The confusion/frustration factor is unfortunately quite high.

@mbest

This comment has been minimized.

Show comment Hide comment
@mbest

mbest Mar 16, 2013

Member

Adding data-bind="value:undefined" to the option seems to fix the problem: http://jsfiddle.net/mbest/SfpVd/2/ Do you think it's enough to solve this?

Member

mbest commented Mar 16, 2013

Adding data-bind="value:undefined" to the option seems to fix the problem: http://jsfiddle.net/mbest/SfpVd/2/ Do you think it's enough to solve this?

@mbest

This comment has been minimized.

Show comment Hide comment
@mbest

mbest Mar 16, 2013

Member

I've attached code changes to make the value binding select the first item if null, undefined, or "" is set.

Member

mbest commented Mar 16, 2013

I've attached code changes to make the value binding select the first item if null, undefined, or "" is set.

@mbest mbest referenced this pull request Mar 16, 2013

Closed

March 2013 iteration #877

rniemeyer added a commit that referenced this pull request Mar 17, 2013

Merge pull request #493 from SteveSanderson/493-select-value-supports…
…-null

value binding on select doesn't respond to null when not using options binding

@rniemeyer rniemeyer merged commit f68ce08 into master Mar 17, 2013

@rniemeyer rniemeyer deleted the 493-select-value-supports-null branch Mar 17, 2013

@mbest mbest restored the 493-select-value-supports-null branch Mar 17, 2013

@rniemeyer rniemeyer deleted the 493-select-value-supports-null branch Mar 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment