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): show doc sidebar on pages with case-sensitive paths #2235

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Jan 21, 2020

Motivation

Since the router configures with case-insensitive paths (by default), we will display the page correctly if the path is written in a different case (creating-Pages -> creating-pages (proper path), and vice versa). Although my personal opinion is that in this case you need to display a 404 error, because this is technically the wrong address, it means that we need to work it out correctly.

However, changing current behavior will result in breaking changes, which is best avoided, so let's leave it as it is. But as noted in issue #2190, in such cases the doc sidebar is not shown. This PR fixes this.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  1. Change or create the id of front matter and filename of any doc page to case-sensitive, eg:
- id: docusaurus-core
+ id: Docusaurus-Core

Make the appropriate changes also in sidebar.js file.

  1. Navigate to docs/docusaurus-core (page with wrong path that we will handle) and docs/Docusaurus-Core (new correct path) - the result should be the same.
    Note that, only when going to the wrong page (docs/docusaurus-core) for a second or so will content of 404 page be displayed (which should be)

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit faef338

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

@bravo-kernel
Copy link
Contributor

bravo-kernel commented Jan 21, 2020

@lex111 you might not be a pro but this is pure magic 💃 , I think many users will be happy with this.

Nevertheless I totally agree with your statement and believe that users should fix root causes. For example, I can imagine that the Netlify redirects from mixed case to lowercase could impact SEO (pure intuition). The same for the 404s.

@lex111 lex111 added the pr: bug fix This PR fixes a bug in a past release. label Jan 21, 2020
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Looks good, many thanks! Question about the renaming of props, otherwise feel free to self-merge.

Add the "tag: breaking" tag if you decide to go ahead with renaming the prop.

@lex111 lex111 added the pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. label Jan 23, 2020
@lex111 lex111 merged commit 42061c6 into facebook:master Jan 23, 2020
@lex111 lex111 deleted the sidebar-case-insensitie-paths branch January 23, 2020 16:07
@lex111 lex111 restored the sidebar-case-insensitie-paths branch April 18, 2020 13:54
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: breaking change Existing sites may not build successfully in the new version. Description contains more details. 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.

5 participants