-
-
Notifications
You must be signed in to change notification settings - Fork 186
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
Replace deprecated title helper to page-title #716
Conversation
@@ -1,5 +1,4 @@ | |||
<HeadLayout /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like deleting <HeadLayout>
resulted in a breaking change for us. Previously, in https://deploy-preview-714--ember-website.netlify.app/, we could see 3 <meta>
tags near the bottom of <head>
:
In https://deploy-preview-716--ember-website.netlify.app/, I'm unable to find these tags.
Now that ember-page-title
no longer depends on ember-cli-head
, I think we'll need to find another way to dynamically create these meta tags.
References:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked for help on Discord: https://discord.com/channels/480462759797063690/486548111221719040/798611445901950986
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@czikarito Thanks for investigating and working on making the pull request.
I think we'll need to find a solution for continuing to create the 3 meta tags. Let me block merging this pull request until we can do so.
@ijlee2 good catch. Maybe we can change only |
Yep, I think it'll be okay to just update the helpers. Thanks for the suggestion! |
5c33dd8
to
a0c6b6f
Compare
@ijlee2 I updated PR to include only helpers change |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI logs look great, thank you again! ✨
👋 @ijlee2, the titleDidUpdate hook PR has been released, so I think that clears the path for updating |
Example integration with ember-cli-head - ember-cli/ember-page-title#201 (comment) |
@bertdeblock @raido Thank you for working on a solution quickly! I'd be happy to try it out right away. |
Resolves #711
Notes
According to https://github.com/ember-cli/ember-page-title#upgrading-notes-for-5x-to-6x I had to remove usage of
HeadLayout
fromapplication.hbs