search

xomaczar / ember-cli-ion-rangeslider

An Ember and ion.RangeSlider integration, packaged as an Ember-cli addon.
MIT License
7 stars 14 forks source link

Remove defaults? #3

Open samselikoff opened 9 years ago

samselikoff commented 9 years ago

The ion rangeslider lib itself documents default behavior when options aren't passed in, and I'd recommend preserving that behavior. The defaults in the mixin yield some surprising behavior. For example I rendered 

{{ember-ion-rangeslider
  type="double"
  min=1
  max=10
  updateTrigger='change'
  from=importanceMin
  to=importanceMax}}

but it didn't work, since from_min and to_min are set to 10. Since the lib itself defaults these to the values of min and max, this component should preserve that behavior.

Thoughts? Would you accept a PR that does this?

xomaczar commented 9 years ago

Agree completely. We should preserve the out of box default options for ion range slider. I would definitely accept the PR that addresses that.

xomaczar commented 9 years ago

@samselikoff Do you have any time to dig in, if not, I can start on a fix.

samselikoff commented 9 years ago

sure, go for it :) i was going to maybe offer an option to opt-in to the defaults, so if people upgrade they can opt-in to backwards compat.

duboff commented 9 years ago

Had the same issue. It's easy enough to work around but would be better not to, or at least to document somewhere that things like to_min / from_min etc should be set most of the time.

xomaczar commented 9 years ago

I just haven't had any time lately. PRs are always welcome. I am trying to find time to fix issues, add unit tests, etc -- emberobserver is giving me a 3/10 --- can't stand it.

duboff commented 9 years ago

I'll see what I can do but also swamped. To be fair, it's an awesome addon, and figuring out this stuff is not too hard. Maybe just updating the readme is enough?