Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure can-value bound input stay in sync with compute #888

Closed
wants to merge 1 commit into from

Conversation

matthewp
Copy link
Contributor

It's possible for a can-value bound input to become out of sync with it's underlying data if that data is a compute with a setter function. The setter function might reject the user's input or set the value to something else entirely.

Previously we were assuming that the compute's new value would always be the same as the input's value. This checks to make sure that the values are the same, otherwise it sets the input's value to reflect the actual value of the compute.

Fixes #887

This change ensures that when the compute value is set due to a user
typing into an input, that the input's value is checked against the new
value and that they are the same. Otherwise it is possible for the
compute to reject the input's value (or set it to something else
entirely) and the input and compute value become out of sync. Fixes canjs#887
// Set the value of the attribute passed in to reflect what the user typed
this.options.value(this.element[0].value);
var newVal = this.options.value(el.value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think can.compute necessarily always returns its current value when being set. I would change this to:

this.options.value(el.value)
var newVal = this.options.value();

@daffl
Copy link
Contributor

daffl commented Feb 11, 2015

Resubmitted as #1436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants