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

fix(v2): use site title if enabled blog-only mode #2259

Merged
merged 1 commit into from
Feb 8, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Jan 31, 2020

Motivation

Resolving of https://docusaurus.canny.io/admin/board/feature-requests/p/how-to-change-title-on-blog-only-mode (see also this comment)

When using Docusaurus as a blog-only mode, the title of the main page is just "Blog", although in this case it is expected to see the site title (specified in the config file).

In addition, as part of this PR, the title of other blog-related pages (tags lists and post tag pages) has been adjusted to make them more suitable for blog-only mode.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Enable blog-only mode and check meta titles.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@lex111 lex111 added the pr: bug fix This PR fixes a bug in a past release. label Jan 31, 2020
@lex111 lex111 requested a review from yangshun January 31, 2020 04:40
@lex111 lex111 requested a review from wgao19 as a code owner January 31, 2020 04:40
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jan 31, 2020
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit da09d5b

https://deploy-preview-2259--docusaurus-2.netlify.com


return (
<Layout title="Blog" description="Blog">
<Layout title={title} description="Blog">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

description="Blog"

By the way, I’m not sure that it’s a good idea to specify a description for this page, since this is optional, and in its current form it is too short.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better as Blog | ${title}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think so, because it was assumed that the title would be completely its own without adding anything else.

@@ -48,7 +48,7 @@ function BlogTagsListPage(props) {
.filter(item => item != null);

return (
<Layout title="Blog Tags" description="Blog Tags">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with Blog Tags here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make the headline "more general", that it was suitable for both the regular using mode and the blog-only. This is all because on other blog pages it is difficult to determine if blog-only mode is enabled. I only had an idea with a global variable on my mind (via webpack), but I decided that I could reduce my efforts and slightly change the title 🤷‍♂️

Should I revert this change?


return (
<Layout title="Blog" description="Blog">
<Layout title={title} description="Blog">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better as Blog | ${title}?

const {
siteConfig: {title: siteTitle},
} = useDocusaurusContext();
const isBlogOnlyMode = metadata.permalink === '/';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this. How does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is wrong? If the blog routeBasePath is equal to / then blog-only mode is used!
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess referencing the permalink field wasn't intuitive to me

@yangshun yangshun merged commit b1e079f into facebook:master Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants