yapplabs / ember-wormhole

Render a child view somewhere else in the DOM.
MIT License
284 stars 68 forks source link

[BUGFIX] If `renderInPlace` is true, destinationElement is ignored #101

Closed cibernox closed 6 years ago

cibernox commented 6 years ago

I found this because I had a component that used ember-wormhole internally like this:

{{#ember-wormhole renderInPlace=renderInPlace destinationElement=destinationElement}}
  {{yield}}
{{/ember-wormhole}}

and it did complain about destinationElement being null, when it wasn't necessary.

lukemelia commented 6 years ago

@cibernox Thanks! It looks like there is a test failure as a result.

cibernox commented 6 years ago

I'll check it in a moment.

cibernox commented 6 years ago

@lukemelia Fixed. As I see it, the failing test was asserting the wrong thing. At least in my mental model I think that renderInPlace=true wins over destinationElement and the test was expecting the wrong behavior.

lukemelia commented 6 years ago

@cibernox I'm with you on that.

cibernox commented 6 years ago

The failures seem unrelated now 🤔

cibernox commented 6 years ago

@lukemelia do you want me to track those errors in master?

lukemelia commented 6 years ago

@cibernox If you have time, I would be even more appreciative of you than I am everytime I use ember-power-select!

cibernox commented 6 years ago

@lukemelia I'll do it tonight.

Before I start, given that canary is 3.0.0, how do you feel about dropping support for Ember 1.13 and make 2.4 the first supported LTS? This is regardless of fixing CI, just thinking ahead.

lukemelia commented 6 years ago

@cibernox I'm open to that.

cibernox commented 6 years ago

@lukemelia Rebased & green

lukemelia commented 6 years ago

Merged. Thank you, Miguel. Sent you an invite to have commit bit, too.

cibernox commented 6 years ago

Thanks! If you could release a new patch version I'll update it in EPS.

lukemelia commented 6 years ago

@cibernox Released as 0.5.4