Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

cibernox
Copy link
Collaborator

@cibernox cibernox commented Dec 8, 2017

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
Copy link
Contributor

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

@cibernox
Copy link
Collaborator Author

cibernox commented Dec 8, 2017

I'll check it in a moment.

@cibernox
Copy link
Collaborator Author

cibernox commented Dec 8, 2017

@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
Copy link
Contributor

@cibernox I'm with you on that.

@cibernox
Copy link
Collaborator Author

cibernox commented Dec 8, 2017

The failures seem unrelated now 🤔

@cibernox
Copy link
Collaborator Author

cibernox commented Dec 8, 2017

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

@lukemelia
Copy link
Contributor

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

@cibernox
Copy link
Collaborator Author

cibernox commented Dec 9, 2017

@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
Copy link
Contributor

@cibernox I'm open to that.

@cibernox cibernox force-pushed the when-rendered-in-place-ignore-destination branch from 00bb8af to f2bc98a Compare December 9, 2017 22:26
@cibernox
Copy link
Collaborator Author

cibernox commented Dec 9, 2017

@lukemelia Rebased & green

@lukemelia lukemelia merged commit ea0fd40 into yapplabs:master Dec 9, 2017
@lukemelia
Copy link
Contributor

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

@cibernox cibernox deleted the when-rendered-in-place-ignore-destination branch December 9, 2017 22:48
@cibernox
Copy link
Collaborator Author

cibernox commented Dec 9, 2017

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

@lukemelia
Copy link
Contributor

@cibernox Released as 0.5.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants