WebReflection / hyperHTML-Element

An extensible class to define hyperHTML based Custom Elements.
ISC License
202 stars 25 forks source link

Latest version 2.1.1 sets value to true if passed an empty value to an attribute #34

Closed pinguxx closed 6 years ago

pinguxx commented 6 years ago

See the example here https://stackblitz.com/edit/js-mjkhqr?file=index.js

WebReflection commented 6 years ago

Please check the PR and let me know if that works.

pinguxx commented 6 years ago

i rather specifically add boolean to my props than the other way around, like this better thanks

WebReflection commented 6 years ago

I think I don't want to rush again this change ... I might go back to original implementation which is consistent in both get or set where maybe setting null or undefined is the only exception that makes sense to remove the attribute, and having a get booleanAttributes() {} similar to observedAttributes would be way better to handle specific boolean cases.

justintaddei commented 6 years ago

This change breaks my app. Anytime someone clears a text input field it is filled with true

Which version was this change introduced? I will need to rollback until it is changed or do a lot of refactoring. Although I don't even know if it's possible to fix the issue unless the change is reverted because anytime I do myInput.value = "" it will be set to true.

How am I to clear an input field now?

WebReflection commented 6 years ago

@justintaddei sorry to hear that. The version without this issue is 2.0.2 but I'll push out soon a patch to fix this (soon as in hopefully few hours max)

WebReflection commented 6 years ago

FYI v2.2.0 fixed this issue and introduced explicit get booleanAttributes() { return []; } at class level.

justintaddei commented 6 years ago

Yay! Thank you. I like this approach a lot better.