-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
feat(blog): Add frontMatter.title_meta
to override title for SEO
#10586
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
It looks like Perhaps |
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.
Almost LGTM but I'd prefer that you don't modify the docs plugin, and you don't add this to blog metadata (only front matter should be enough, it's temporary so it's less code to remove in the future)
The tests are fine and did not use title_meta
, but if you add it to metadata, you have many tests and snapshots to modify. Run yarn test --watch
locally until tests pass. Once you remove the metadata, you probably don't need to change the existing tests anymore.
However you need to add frontMatter.title
to the validation schema and add an unit test for it.
Also need to write docs
packages/docusaurus-plugin-content-blog/src/plugin-content-blog.d.ts
Outdated
Show resolved
Hide resolved
packages/docusaurus-plugin-content-blog/src/plugin-content-blog.d.ts
Outdated
Show resolved
Hide resolved
packages/docusaurus-plugin-content-docs/src/plugin-content-docs.d.ts
Outdated
Show resolved
Hide resolved
/** | ||
* Used to generate the <head><title> and <meta>. | ||
*/ | ||
title_meta?: string; |
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.
remove
packages/docusaurus-theme-classic/src/theme/BlogPostPage/Metadata/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/DocItem/Metadata/index.tsx
Outdated
Show resolved
Hide resolved
I think that it would be helpful to decide what is the final desired behaviour, and try to implement it incrementally, without having to reverse changes. Before submitting the PR I did some experimentation and, from my limited point of view, I covered all use cases that I could imagined, but submitted only part of the changes, as a first step. First, do you agree that we should do our best to have a consistent solution for both docs and blog posts? Then, do you agree that we should have at least one solution for defining shorter titles, to be passed for SEO usage, at the same time with longer titles, to be displayed in the web, and this solution once implemented, should be documented and remain in the code? As far as I can tell, I can see two solutions:
To achieve this, the current changes achieve this; if you think that some can be simplified, ok, we can simplify them.
To achieve this, in addition to the suggested changes, two more lines need to be changed, the The blog title should be computed as (the comment is the current line):
The docs title should be computed similarly:
If we implement both cases, I think that it is better to have two separate variables, one to store the title shown on the web pages ( For consistency reasons, I would use the same mechanism for both docs and blog posts. The only detail not covered by the above solution is filtering out markdown syntax from the H1 header, but I think that this can be added in a later step. If I totally misunderstood the subject and you have a different approach in mind, perhaps we should wait for the final solution and avoid adding now a temporary workaround to be later removed. For my use case I would probably go for using Please advise how to proceed. |
I don't want to introduce this to docs because we don't need it, and this would be another thing to remove in the future. I prefer a temporary small inconsistency than a larger temporary but consistent change. Also, reversing the title logic is a breaking change. I don't want to do such a breaking change when we can solve the problem in a retrocompatible way without disrupting existing usage and asking users to modify their md files. The Please just add |
Done. |
Please note a small naming inconsistency, the docs frontMatter properties |
I thought this was already possible by using an |
@alvinometric afaik it's not the case for blog posts (only docs), and I explained why here: #10578 (reply in thread) |
I think it's fine and better this way. We want to "group" things by concerns with a prefix. |
frontMatter.title_meta
to override title for SEO
Did you document this somewhere? Or it'll remain undocumented? |
Yes I edited your PR |
I see. But the new text does not mention that this is a temporary solution. |
Pre-flight checklist
Motivation
The new frontMatter property allows to generate shorter titles in the
<head>
section, for both docs & blogs, while keeping the existing long titles for the page headers & other places where blog post titles are used.Test Plan
Test links
Related issues/PRs