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

Fixing template property deprecation, so the addon can work with latest ember v2.10.0 #88

Merged
merged 6 commits into from
Jan 10, 2017

Conversation

waleedq
Copy link
Contributor

@waleedq waleedq commented Dec 13, 2016

Hello,

It appears that the component's template property is deprecated from v1.x api but there was no any deprecation messages about using it, and it have been replace with the layout property. to fix this issue in v2.10.0 the partial helper should be used to render the given template inside the dialog layout

this PR should fix this issue #87

Thank You,

@ivandokov
Copy link
Contributor

Maybe you should also change the README file to show the layout changes

@ajile
Copy link
Contributor

ajile commented Dec 15, 2016

Recently I do not have spare time to review and apply PR. I will try to do that tomorrow. I'm sorry about it.

@ajile
Copy link
Contributor

ajile commented Dec 16, 2016

I tried to run some examples from the dummy (in russian version) and found a problem - partial helper is not working with precompiled template. Occurs an internal exception:

Error while processing route: index Could not find a partial named "[object Object]"
Error: Could not find a partial named "[object Object]"

That happens because the partial helper expects a string, but gets an object (precompiled template). You can reproduce it so:

import Ember from "ember";
import hbs from 'htmlbars-inline-precompile';

export default Ember.Controller.extend({
  init() {
    this.get("dialog").alert(hbs`Hello world`);
    return this._super(...arguments);
  }
});

I need time to research Glimmer to find way to fix it. For now I don't know how Glimmer works.

@ajile
Copy link
Contributor

ajile commented Dec 16, 2016

I've reproduced problem on ember-twiddle.

@ajile
Copy link
Contributor

ajile commented Dec 19, 2016

After talking with community in slack, I've found solution to paste prerendered template into the presenter's body. I've created one more component (presenter-body) that gets template as layout. From now if template is a string - it's introduced into the presenter's body as a partial, otherwise it introduced as a presenter-body component.

@waleedq, could you review and accept PR waleedq#6 if all fine.

@@ -7,9 +7,6 @@ moduleFor('service:dialog', 'Unit | Service | dialog', {
needs: ['component:presenter'],
setup() {
this.service = this.subject();
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's removed by mistake, i suppose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how git displays the change. There is one } below. The code is fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that the teardown method was removed in this commit. I pulled commit into the local machine. There is really no teardown.

@waleedq
Copy link
Contributor Author

waleedq commented Jan 10, 2017

@ajile Sorry for being late, i've tested the PR and it works totally fine it's merged now, please check it out and let me know

@ajile
Copy link
Contributor

ajile commented Jan 10, 2017

@waleedq thank you! Everything works well. I'm going to merge PR and release stable version.

@ajile ajile merged commit 9f07e31 into wheely:master Jan 10, 2017
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.

3 participants