zestia / ember-select-box

:capital_abcd: A faux select box for Ember apps
MIT License
64 stars 14 forks source link

Ember compatibility #17

Closed ctjhoa closed 5 years ago

ctjhoa commented 5 years ago

Hi,

According to your package.json versionCompatibility this add-on is suppose to be compatible with ember >= 2.3.0

I tried to integrate it and I faced multiple issues using ember 2.4.6. So I decided to clone this repo, configured ember-try for ember 2.4 and here are my first test result:

# tests 407
# pass  226
# skip  0
# fail  181

I noticed that many errors came from missing polyfill. So I decided to add ember-factory-for-polyfill & ember-assign-polyfill. Then I reran the test:

# tests 391
# pass  244
# skip  0
# fail  147

Again most tests fails are related to one error:

Uncaught Error: Assertion Failed: You must use Ember.set() to set the `element` property (of <dummy@component:select-box::ember995>) to `[object HTMLDivElement]`.

I narrowed down this issue to this line https://github.com/zestia/ember-select-box/blob/master/addon/templates/components/select-box.hbs#L40 . If I completely remove this line element=this.element here are my tests:

# tests 322
# pass  309
# skip  0
# fail  13

Not perfect but usable as failing tests aren't related to the add-on core functionality. This line of code has been introduced in this commit 2f95ac642e508a19401b6a21726c11cda2700dd2 (for the 4.0.0 version of ember-select-box)

So what's the real compatibility with previous ember versions of this add-on? Will you accept PR which fix compatibility? Even if it breaks current api?

You can find my work here: https://github.com/ctjhoa/ember-select-box

Thanks

amk221 commented 5 years ago

Thanks for this, and sorry if the backwards compat broke, I'll take a look and see if I can fix this.

amk221 commented 5 years ago

Hmm, I just tried it with LTS 2.12 and it works ok. Travis also reports the same.

ctjhoa commented 5 years ago

I'm using ember 2.4.6

amk221 commented 5 years ago

Ah balls, sorry it's because I moved the addon to use named arguments. Which requires Ember 2.10+

ctjhoa commented 5 years ago

ok so versionCompatibility in package.json should be ember >= 2.10.0 ?

amk221 commented 5 years ago

Yes, I'm really sorry, I should have spotted before introducing the named arguments polyfill.