max-mapper / yo-yo

A tiny library for building modular UI components using DOM diffing and ES6 tagged template literals
1.33k stars 65 forks source link

do not copy form values if value attribute is set #68

Closed abradley2 closed 7 years ago

abradley2 commented 7 years ago

*Updated based on some discussion in this issue

Because an input that looks like this <input type='text'/> will still have a value of "", the el.value method can't be used to distinguish between how it should update different than something like <input type='text' value="" />

Use case:

Between updates: <input type='text' /> = will copy over the value, because hasAttribute('value') will return false even after user input

<input type='text' value=${val} /> will set the value to whatever val is, even if it is an empty string, because hasAttribute('value') will return true

This relates to the issue here: https://github.com/maxogden/yo-yo/issues/66

toddself commented 7 years ago

This sounds like a valid fix. Are we sure hasAttribute works the same way in older browsers? Speaking of, @maxogden is there a cut-off for browser support for this library?

yoshuawuyts commented 7 years ago

@toddself iirc so far for choo we've been targeting IE10+ because anything below that is like 1% market share - which is less than mobile Opera and most people don't really account for that - can't speak for yo-yo ofcourse, but I would assume it'd follow something pragmatic (:

Mr0grog commented 7 years ago

FWIW, hasAttributeNS() is supported in IE as far back as 9. In IE8, you’d have to fall back to either hasAttribute() or the attributes property.

Mr0grog commented 7 years ago

Errr, sorry, I had misread/miscarried some conversation from #66 over here; I see this isn’t using hasAttributeNS(). Anyway, hasAttribute() works at least as far back as IE8. I don’t have an older IE to test.

harrygr commented 7 years ago

This looks like a needed fix - any chance this could get merged?

kristoferjoseph commented 7 years ago

@yoshuawuyts @maxogden @shama Can we merge this pretty please? Thank you kindly.

yoshuawuyts commented 7 years ago

v1.4.1