observe title and titleToken

[ssendev]

allows to update the title without route transition. this solves #9

[ssendev]

travis fails because registry.register is undefined in the router:main tests on canary. all other tests pass

[ssendev]

ok apparently observing currentModel or controller is not supported emberjs/ember.js#9767 this causes transitioning to the same route with a different model to fail if a computed property is used that uses them.

volatileCurrentModel: Ember.computed(-> @get 'currentModel').volatile()
titleToken: Ember.computed.alias 'volatileCurrentModel.title'

seems to be a viable workaround

[ssendev]

@kimroen any interest in this? if so i could rebase.

[kimroen]

I am very interested in the feature itself, but I have not yet sat down and looked at how good of a solution this is. I still plan to do so, though, so if you have the chance to rebase, I'd appreciate it. Thanks!

[ssendev]

volatileController and volatileCurrentModel are not the best names but i couldn't think of better ones.

[kimroen]

this causes transitioning to the same route with a different model to fail if a computed property is used that uses them.

Could you elaborate on this?

I seem to be able to make it work just fine by using controller.model as the dependent key, in both cases where this is used in the tests:

title: Ember.computed('controller.model.donations', function() {
  return `Donations: ${this.get('controller.model.donations')`;
})

[kimroen]

This comment doesn't make much sense here.

[kimroen]

All right, I've looked through this now, and I have some thoughts:

In general, I think the idea and implementation is pretty solid. Instead of collecting the tokens in the bubbling action, you collect the routes, and then loop through them and add observers (etc) once the route with a title is reached. Good idea :)

Observing things

Right now, the titleToken of all the routes up to the one that has a title will have an observer attached to it.

titleToken can either be a primitive data type or a function. The function can be either a normal function or an Ember.ComputedProperty. To keep things simple (both to write and to understand), I think it makes more sense to only look for changes to computed properties, and not anything else.

Computed titles

This is a new thing with this change, but it seems straight forward enough, I think. The same thing as above goes for this.

volatileController and volatileCurrentModel

The main feature here is that you can now provide a computed property as your title or titleToken, and it will actually update the document title if your computed property is invalidated.

I don't think it makes sense for this library to define these properties. How the user retrieves the properties they want to use for their titles should be up to them.

Thanks again for this, and sorry about the delayed response.

[ssendev]

volatileController

i added a test which fails fails if volatileController.model is replaced with controller.model or currentModel. interestingly it also failed with volatileCurrentModel so i removed it. this is the reason i included it by default, because first it looks like it is working but then it suddenly blows up (so it should be clearly documented how to do it the right way, otherwise people will be frustrated). and everyone using computed properties is gong to want to observe properties on the controller since the route doesn't have anything interesting. which means on every route volatileController would have to be reimplemented.

observing things

the computed property is transparently resolved on the first get so the function check wouldn't catch it. this also makes it hard to argue if there is a computed property or not since it could mean it just resolved to null at the moment but might change in the future. it might also be a property which doesn't exist at the moment but is set later. so i think observers should always be set up. (the only exception could be if it is a function but even then it could be a computed property that returned a function. or the function could be reassigned with set). i also think it would only make things more complex since right now it just adds observers on all routes and this would add another check. and since routes aren't changed frequently and the nesting's probably limited to a view levels these ~3 extra observers don't harm performance.

[kimroen]

I had a case of paralysis by analysis combined with burnout here. Really sorry about the lack of a response.

I've been having some trouble putting words to the issues I have with this binding to the controller, but I believe it comes down to violating the principle of Data Down, Actions Up in Ember—it doesn't feel right.

There is probably a good reason the controller is not observable by default, like @mixonic mentioned in his comment.

There are some parallels between this change and the query parameters feature in Ember. They were moved to be partly in the controller, but the plan is to move them back to the route once Routable Components are a reality. Some of the problems we are discussing here will have to be tackled in query params as well, so I’m a bit curious as to what the design of that will be.

Regarding observing, I agree with your conclusion, but some comments:

the computed property is transparently resolved on the first get so the function check wouldn't catch it.

To clarify, right now we’re doing a Ember.get of the property, which will trigger the computed property, but we could get the property the normal way and check what it is instead if we wanted to.

// Ember getter
var titleToken = get(route, 'titleToken');

// Normal property assignment
var titleToken = route.titleToken;

I don’t think it’s correct that we couldn’t check, but I still agree that the complexity of setting it up and handling it is probably not worth it.

Again, sorry for the delay and lack of actionable feedback here, but I felt it was better to share this incomplete thought than keeping quiet.

[elwayman02]

This should probably not be included in the addon...it's an implementation detail of the consuming application, and it's not the only way a token might change. I don't think the addon itself should provide this.

[elwayman02]

I'm hesitant to introduce observers, since the message from the core team has generally been to stop using observers if at all possible. Perhaps there is another way?

[grapho]

I feel edgy about the observers as well, maybe unjustifiably so.

One of the most beautiful things about this addon is its simplicity. Serialize app state to title.

i am tl;dr; all the details and the usecase for this PR.. so at risk of sounds completely clueless... perhaps this addon can provide a set of helpers and or mixin that allows use to just set document.title without affecting the default functionality?

Update:
Looked at more code. It seems like each route that has a title would get an observer attached? So does that mean my app with 71 possible routes, will have potentially 71 observeables added?

[GCheung55]

Has anyone taken a look at https://github.com/tim-evans/ember-page-title ? While the purpose of ember-page-title is the same, it's using template helpers to do it. Anything ember-cli-document-title could benefit from?

[kimroen]

Has anyone taken a look at https://github.com/tim-evans/ember-page-title ? While the purpose of ember-page-title is the same, it's using template helpers to do it. Anything ember-cli-document-title could benefit from?

I talked with @tim-evans at EmberConf about merging the two libraries actually. I'm still open to that, it's just not completely clear what that would look like and how one could go about doing it in practice. The two libraries have pretty different ideas of who owns the title of the document, and that difference is significant for how we solve this issue.

The problem in this PR is not really how to update the title at all, the sticking point is that the most common use case for observable titles is to depend on the model of the route, and the route doesn't have an observable path to the current model. After the transition is over, the model has been handed over to the controller. In ember-page-title this isn't a problem, because the context of the template is the controller.

[tim-evans]

Agreed, @kimroen. I feel that having a bit of guidance as to what the future of Ember's layers will look like may help designing the happiest path forward.

It's tough to design something that makes everyone happy, which I think is why we have two solutions at the moment 😅

[snewcomer]

Closing due to outdated. Let us know if 1.0 has any issues!