Formalize two-way binding with selects. #2027

Closed
justinbmeyer opened this Issue Oct 24, 2015 · 11 comments

Comments

Projects
None yet
5 participants
@justinbmeyer
Contributor

justinbmeyer commented Oct 24, 2015

Using the new two-way binding syntax {($value})="key", the parent scope value should be updated assuming the parent value is undefined.

However, this makes it difficult to get the nice behavior of showing an empty select when the scope is undefined. And once the value is selected, you can no longer select the empty option.

This is still possible, but tricky. I'd like a way to make it easier.

@daffl daffl added this to the 2.3.1 milestone Oct 26, 2015

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Oct 26, 2015

Contributor

I don't think this is fixable. A select field like

  <select id="test">
    <option>One</option>
  </select>

When set like this:

var element = document.getElementById('test');
element.value = '';

Will always show an empty select in Chrome/Safari etc. and the first item in Firefox.

Contributor

daffl commented Oct 26, 2015

I don't think this is fixable. A select field like

  <select id="test">
    <option>One</option>
  </select>

When set like this:

var element = document.getElementById('test');
element.value = '';

Will always show an empty select in Chrome/Safari etc. and the first item in Firefox.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 26, 2015

Contributor

Yarg ... that's dumb. So for this to work in all browsers, we'd have to add an empty ?

Contributor

justinbmeyer commented Oct 26, 2015

Yarg ... that's dumb. So for this to work in all browsers, we'd have to add an empty ?

@DesignByOnyx

This comment has been minimized.

Show comment
Hide comment
@DesignByOnyx

DesignByOnyx Oct 27, 2015

Contributor

If you set the selectedIndex to -1 it will clear out the element, even in Firefox:

http://jsfiddle.net/3xyxpcqo/

It seems that all browsers report the value as the first option in the list by default, and then an empty string when the selected index is set to -1.

Contributor

DesignByOnyx commented Oct 27, 2015

If you set the selectedIndex to -1 it will clear out the element, even in Firefox:

http://jsfiddle.net/3xyxpcqo/

It seems that all browsers report the value as the first option in the list by default, and then an empty string when the selected index is set to -1.

@matthewp matthewp self-assigned this Oct 28, 2015

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Oct 29, 2015

Contributor

Normally when two way binding we set initial values based on which has a value. In this case the parent doesn't have a value but the child does, so we are making an exception for selects and saying that their value doesn't matter?

Contributor

matthewp commented Oct 29, 2015

Normally when two way binding we set initial values based on which has a value. In this case the parent doesn't have a value but the child does, so we are making an exception for selects and saying that their value doesn't matter?

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Oct 29, 2015

Contributor

More clearly, for selects we are ignoring their value and forcing parent->child initialization. Is this right?

Contributor

matthewp commented Oct 29, 2015

More clearly, for selects we are ignoring their value and forcing parent->child initialization. Is this right?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Oct 29, 2015

Contributor

This is a special case because FF forces it to be.

Ideally, we wouldn't do special casing here. I wonder if it's possible to tell if the value was actually set, or just defaulted to the first item.

Sent from my iPhone

On Oct 29, 2015, at 8:23 AM, Matthew Phillips notifications@github.com wrote:

Normally when two way binding we set initial values based on which has a value. In this case the parent doesn't have a value but the child does, so we are making an exception for selects and saying that their value doesn't matter?


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Oct 29, 2015

This is a special case because FF forces it to be.

Ideally, we wouldn't do special casing here. I wonder if it's possible to tell if the value was actually set, or just defaulted to the first item.

Sent from my iPhone

On Oct 29, 2015, at 8:23 AM, Matthew Phillips notifications@github.com wrote:

Normally when two way binding we set initial values based on which has a value. In this case the parent doesn't have a value but the child does, so we are making an exception for selects and saying that their value doesn't matter?


Reply to this email directly or view it on GitHub.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Oct 29, 2015

Contributor

I don't think it's possible to set a select's value as undefined so the parent compute is always going to be initialized as an empty string if we do this.

Contributor

matthewp commented Oct 29, 2015

I don't think it's possible to set a select's value as undefined so the parent compute is always going to be initialized as an empty string if we do this.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Oct 29, 2015

Contributor

Scratch that, if we initialize this special case before bindings are set up the parent compute's value won't be updated. This sucks though.

Contributor

matthewp commented Oct 29, 2015

Scratch that, if we initialize this special case before bindings are set up the parent compute's value won't be updated. This sucks though.

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Oct 29, 2015

Contributor

This changes the behavior that was added for:

Contributor

matthewp commented Oct 29, 2015

This changes the behavior that was added for:

@matthewp matthewp removed their assignment Oct 29, 2015

@justinbmeyer justinbmeyer modified the milestones: 2.3.2, 2.3.1 Oct 29, 2015

@justinbmeyer justinbmeyer self-assigned this Nov 2, 2015

@justinbmeyer justinbmeyer changed the title from Firefox showing first option as selected when there is no value. to Formalize two-way binding with selects. Nov 2, 2015

@Alfredo-Delgado

This comment has been minimized.

Show comment
Hide comment
@Alfredo-Delgado

Alfredo-Delgado Nov 11, 2015

Contributor

Working on this, I'm not sure that the case where a key in a parent scope is undefined should be our concern.

e.g. current behavior (consistent across browsers)

var map = new can.Map({ });
equal(frag.firstChild.selectedIndex, 0, 'undefined <- {($first value)}: selectedIndex = 0');

What isn't consistent is what happens when the key's value doesn't match one of the select's options:

map.attr('key', 'does not match one of the options');
equal(frag.firstChild.selectedIndex, -1, 'selectedIndex = -1');

This passes in Chrome, but fails in Firefox (selectedIndex === 0).

This branch addresses this much. I added the fix in attr.set because it seems like a universal problem. Like @matthewp mentioned above, this new behavior conflicts with the change done at c705aa6

Contributor

Alfredo-Delgado commented Nov 11, 2015

Working on this, I'm not sure that the case where a key in a parent scope is undefined should be our concern.

e.g. current behavior (consistent across browsers)

var map = new can.Map({ });
equal(frag.firstChild.selectedIndex, 0, 'undefined <- {($first value)}: selectedIndex = 0');

What isn't consistent is what happens when the key's value doesn't match one of the select's options:

map.attr('key', 'does not match one of the options');
equal(frag.firstChild.selectedIndex, -1, 'selectedIndex = -1');

This passes in Chrome, but fails in Firefox (selectedIndex === 0).

This branch addresses this much. I added the fix in attr.set because it seems like a universal problem. Like @matthewp mentioned above, this new behavior conflicts with the change done at c705aa6

justinbmeyer added a commit that referenced this issue Nov 12, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 12, 2015

Contributor

@Alfredo-Delgado I checked out your solution after I did mine (in bindings). Yours seems better, but some tests of mine did not pass. My pull request is here: #2076. @daffl If it passes, lets get that in and we can investigate making this work directly in attr later.

Contributor

justinbmeyer commented Nov 12, 2015

@Alfredo-Delgado I checked out your solution after I did mine (in bindings). Yours seems better, but some tests of mine did not pass. My pull request is here: #2076. @daffl If it passes, lets get that in and we can investigate making this work directly in attr later.

@daffl daffl closed this in #2076 Nov 13, 2015

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