component / reactive

Tiny reactive template engine
382 stars 48 forks source link

fix data bindings on input/textarea tag. fix #133 #134

Closed poying closed 10 years ago

poying commented 10 years ago
defunctzombie commented 10 years ago

Not a fan of the way this is written syntax wise. Use an if statement to determine the function first (and set that to a variable) then pass that into this.change(). The ternary is hard to read here and has not saved any lines.

Needs tests.

Once those things are done. Please collapse into one logical commit and then I will merge :) This is a good fix.

vendethiel commented 10 years ago

:+1:

defunctzombie commented 10 years ago

I tried this locally. input.setAttribute('value', value) seems to work just fine. Did you find an instance where this was not the case? I can comment out your special handling code for INPUT and tests pass. Likewise, I can comment out your special handling of TEXTAREA in data-text and tests pass.

Did this fail in a specific browser? If not, then there is no reason to fix what is not broken.

poying commented 10 years ago

It works at the first time, and then it failed.

133

screencast: http://youtu.be/7jfJA9n1pKo

defunctzombie commented 10 years ago

Can we make tests that reproduce this? On May 13, 2014 9:57 PM, "Po-Ying Chen" notifications@github.com wrote:

It works at the first time, and then it failed.

133 https://github.com/component/reactive/issues/133

video: http://youtu.be/7jfJA9n1pKo

— Reply to this email directly or view it on GitHubhttps://github.com/component/reactive/pull/134#issuecomment-43034537 .

poying commented 10 years ago

@defunctzombie

var el = domify('<input data-value="value" />');
var view = reactive(el, { value: '' });
el.value = 'old value';
view.set('value', 'value');
assert(el.value == 'value');
defunctzombie commented 10 years ago

Thanks for the fixes and following up to get this resolved.