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

Fix #104 Toggling renderInPlace #107

Merged
merged 1 commit into from
Sep 20, 2018

Conversation

dnachev
Copy link

@dnachev dnachev commented Jun 13, 2018

After the change in #101, the wormhole cannot toggle from render in place to render elsewhere, because destinationElement CP is never read and therefore the observer will never fire when renderInPlace has changed. (Issue #104).

I'm little unclear why we need the change from #101, but presumably we want to specify an explicit element to be used as a wormhole rather than selector. In this case, we will need to a new computed property (private) to capture all possible parameters, which ember-wormhole component can receive and observe them for changes.

If there are other reasons, we can discuss a different fix for the issue.

@nlfurniss
Copy link
Contributor

nlfurniss commented Jun 14, 2018

@dnachev you can fix the travis issues with this: ember-cli/ember-fetch@0a817ff

@dnachev
Copy link
Author

dnachev commented Jun 14, 2018

Thanks @nlfurniss . Trying it out.

maxfierke added a commit to sir-dunxalot/ember-tooltips that referenced this pull request Sep 11, 2018
Works around broken renderInPlace toggling in ember-wormhole (See yapplabs/ember-wormhole#107 for more info)
@fivetanley
Copy link

@lukemelia apologies for the direct ping; do you think you or another yapper have some cycles to review+release this?

@lukemelia lukemelia merged commit ae40fcb into yapplabs:master Sep 20, 2018
@lukemelia
Copy link
Contributor

@fivetanley For you, anything :D -- released as 0.5.5

@fivetanley
Copy link

@lukemelia <3 quickest merge in the west. thanks!

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

4 participants