kimroen / ember-cli-document-title

Adding document title behaviour to your ember app
MIT License
216 stars 61 forks source link

Fix deprecation by replacing _actions with actions #22

Closed lucas-clemente closed 9 years ago

kimroen commented 9 years ago

I'm not sure it's as straight forward as just replacing it (because we are reopening the class), but I'll take a look some time in the coming week unless you do first :)

lucas-clemente commented 9 years ago

I see. It works for my (limited) scenario, but I don't understand the details ;)

gpoitch commented 9 years ago

This works fine as long as your using 2.0.0 https://github.com/emberjs/ember.js/pull/12028

kimroen commented 9 years ago

Looks like two different errors here: Not using `_actions´ doesn't work with the way we reopen the routes (or something else is wrong) in versions prior to 2.0.0. Also, on the release-version, there is a error in the test:

beforeEach failed on it sets document title on the renderer:-dom if present: 'undefined' is not a function (evaluating 'registry.register('location:none', Ember['default'].NoneLocation)')
rwjblue commented 9 years ago

You need to detect the merged property name, and use that. For 1.x it will be _actions and 2.x it will be actions. There may be an easier way, but this is what I whipped up:

var mergedActionPropertyName = (function() {
  var routeProto = Ember.Route.proto();
  var mergedProps = routeProto.mergedProperties;
  for (var i = 0, l = mergedProps.length; i < l; i++) {
    var property = mergedProps[i];

    if (property === 'actions') {
      return 'actions';
    } else if (property === '_actions') {
      return '_actions';
    }
  }
})();

var routeProps = {};
routeProps[mergedActionPropertyName] = {
  // actions here
};

App.IndexRoute = Ember.Route.extend(routeProps);

Demo: http://emberjs.jsbin.com/huxuwu/1/edit?html,js,output

rwjblue commented 9 years ago

@kimroen - https://github.com/kimroen/ember-cli-document-title/pull/25 should address the error with registry.register.

kimroen commented 9 years ago

Working on this, but just as a note: the functionality itself does seem to be working without any changes.

kimroen commented 9 years ago

Thanks for contributing, and thanks to @rwjblue for the great suggested fix. Releasing a new version shortly which should take care of this.

lucas-clemente commented 9 years ago

Thanks for fixing it, and sorry for not working with you to improve my initial fix. I was caught with other stuff :S

kimroen commented 9 years ago

@lucas-clemente Don't worry about it — I know the feeling :)