-
Notifications
You must be signed in to change notification settings - Fork 12
Fix: ember-page-title errors when head format template contains <title>
#3933
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
base: main
Are you sure you want to change the base?
Fix: ember-page-title errors when head format template contains <title>
#3933
Conversation
Preview deployments |
0c021be to
429efc7
Compare
|
@codex please review |
|
Is there still a conflict between ember-page-title updating the title and dynamically updated "head" formats updating the page title? If so, which one wins? |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ca659d5a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (headHTML != null) { | ||
| headFragments.push(headHTML); | ||
| headFragments.push(this.ensureSingleTitle(headHTML)); | ||
| } | ||
|
|
||
| if (scopedCSS != null) { |
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.
Preserve default title when only scoped CSS is injected
Because the default <title> was moved into the data-boxel-head-* region, calling injectHeadHTML will remove it. In this block you only add a title when headHTML != null, so when a card has no head template but does have scoped CSS, the injected head becomes just the <style> block and the page ends up with zero <title>. This is likely for cards that define scoped CSS but no head format. Consider adding the default title whenever you inject head content without a title, or explicitly include it when headHTML is null but scopedCSS is present.
Useful? React with 👍 / 👎.
Previously, the default
<title>Boxel</title>lived outside the injectable head zone inindex.html, while the realm-server and host mode inject head HTML betweendata-boxel-head-startanddata-boxel-head-end. Enabling the title in the head template would have added a second title, so it stayed commented out. This PR moves the default title into that injectable zone and re-enables it in the head template so injected head replaces the default instead of duplicating it. For custom head templates that omit a title, the default "Boxel" is added automatically so the document always has exactly one<title>.