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

feat(v2): Add themeConfig.noIndex option #3528 #3573

Merged

Conversation

hamzahamidi
Copy link
Contributor

Motivation

This PR is intended to add the noIndex option which was present in V1: https://docusaurus.io/docs/en/site-config#noindex-boolean
More details in the correspondent issue #3528

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

The feature is to add a noIndex option in docusaurus.config.js. If true <meta name="robots" content="noindex" />
will be added to the template.

image

Related PRs

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Oct 11, 2020

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit 691b703

https://deploy-preview-3573--docusaurus-2.netlify.app

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 11, 2020
@slorber slorber linked an issue Oct 12, 2020 that may be closed by this pull request
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, looks like a good start.

Instead of adding the noindex meta from themes code, what about injecting the tag in a generic way directly in core (like, through the SSR template). This way we are sure that this feature work consistently across all themes, including themes built by users/community

Also, we have a migration cli. If v1 had the noIndex option, that would be nice if running the migration cli did output a v2 website also having the noIndex option. You can test this migration with yarn test:v1Migration:migrate

Added some code comments too.

packages/docusaurus-types/src/index.d.ts Outdated Show resolved Hide resolved
packages/docusaurus-types/src/index.d.ts Outdated Show resolved Hide resolved
website/docs/api/docusaurus.config.js.md Outdated Show resolved Hide resolved
website/docusaurus.config.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

LGTM, just need to remove the noIndex setup for development mode and good to merge, thanks

packages/docusaurus-migrate/src/types.ts Outdated Show resolved Hide resolved
packages/docusaurus/src/commands/start.ts Outdated Show resolved Hide resolved
website/docs/api/docusaurus.config.js.md Outdated Show resolved Hide resolved
@hamzahamidi hamzahamidi force-pushed the feat/v2/add-theme-config-no-index branch 2 times, most recently from dd504a5 to cc0d06e Compare October 13, 2020 18:07
@slorber
Copy link
Collaborator

slorber commented Oct 14, 2020

Thanks @hamzahamidi , that looks nice ;)

@slorber slorber merged commit e0c644e into facebook:master Oct 14, 2020
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Oct 14, 2020
@hamzahamidi hamzahamidi deleted the feat/v2/add-theme-config-no-index branch October 14, 2020 12:16
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: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2: Add themeConfig.noIndex option
5 participants