knownasilya / ember-toggle

Checkbox based Toggle Switches for Ember
http://knownasilya.github.io/ember-toggle/
MIT License
112 stars 52 forks source link

Base toggle not using boolean values #28

Closed yankeeinlondon closed 8 years ago

yankeeinlondon commented 9 years ago

I would have assumed unless you used the on or off computed properties that the value of the control would be a boolean true/false but instead I'm getting the capitalized text "True" and "False" for the bound value.

yankeeinlondon commented 9 years ago

In the mean time, setting the on and off properties doesn't seem to work either:

image

Uncaught TypeError: this.get(...).indexOf is not a function

yankeeinlondon commented 9 years ago

Was going to send you a PR but was having problems with getting you addon running locally (CSP problem of some sort). Anyway above if you just had something like:

 const {on,off} = this.getProperties('on','off')
 const onState = Ember.typeOf(on) === 'string' ? on.split(':')[0] : on;
 const offState = Ember.typeOf(off) === 'string' ? off.split(':')[0] : off;

I haven't tested it but it should work.

knownasilya commented 9 years ago

Thanks for the issue & the PR. Still working on converting to the new API so some errors will be trailing around for a bit (need to write tests).

yankeeinlondon commented 9 years ago

Good luck. Out of interest, which API are you referring to? Addon API for Ember-CLI? I had thought the only breaking change recently was for authors using CSS preprocessors.

knownasilya commented 9 years ago

No, API of this addon, i.e. data down, actions up. See https://github.com/knownasilya/ember-cli-toggle/pull/27

yankeeinlondon commented 9 years ago

Ok. My feeling is that two-way binding (the good old fashioned way) is appropriate for low level input controls that in turn will be used by components in composition. For larger grained components I see lots of value in DDAU. What do you think?

I guess I mention this because I a toggle control should still retain the two-way binding option. No issue with allowing for an action-up API in addition too but my guess is that there will more use-cases for the traditional approach with this sort of component.

Personally I'm looking forward to 2.1 hits (or whenever the angle-bracket components hit) to really go whole hog into the one-way binding aspects of DDAU as the current process is a little too much work and if I'm not mistaken in the 2.1+ world you won't have to change anything it instead just becomes a static design-time configuration for which the container (rather than the addon) is given the optionality.

knownasilya commented 8 years ago

Is this still an issue in the latest release?

yankeeinlondon commented 8 years ago

I'd trust you if you said it was. :)

I'm super heads down on other things right now so won't be able to check right now.

knownasilya commented 8 years ago

I think I need to write up some tests.. it's gotten much easier