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

Move title tag cleanup to page-title-list service #117

Merged
merged 1 commit into from
Jan 3, 2018
Merged

Move title tag cleanup to page-title-list service #117

merged 1 commit into from
Jan 3, 2018

Conversation

mydea
Copy link
Contributor

@mydea mydea commented Jan 3, 2018

When using engines, each engine might run the instance initializer, leading to the title tag being removed. This is related to #112, but also to ember-fastboot/ember-cli-head#41.

This PR removes the instance initializer, and instead removes any existing title tags in the service's init() method.

Since the service is shared between engines, this will only be run once.

When using engines, each engine might run the instance initializer, leading to the title tag being removed.
@mydea
Copy link
Contributor Author

mydea commented Jan 3, 2018

If ember-fastboot/ember-cli-head#45 is merged & released, an even nicer solution would be to overwrite the head-layout component, and do the tearing down of the title in it's init() method. This would ensure that the app is never without a title. But this could be done at a later time, I guess. This would also address #116.

@tim-evans
Copy link
Contributor

Thanks for the PR, @mydea ! This looks good to me- I believe the test suite already covers for this. I'll verify on my end 👍

@tim-evans tim-evans merged commit d536b34 into ember-cli:latest Jan 3, 2018
@tim-evans
Copy link
Contributor

Released in 4.0.2

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.

2 participants