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

docs(v2): Move sidebar content to its own page #3899

Merged
merged 4 commits into from
Dec 11, 2020

Conversation

ArtFlag
Copy link
Contributor

@ArtFlag ArtFlag commented Dec 9, 2020

Motivation

Related to #3893 and #3894.

TOCs or sidebars are extremely important in any docs website.

In the current version, the sidebar info is inside the intro page and the sidebar sections are from H3 and deeper, meaning they do not appear in the right-hand TOC making it difficult to browse and understand.

With this PR, the sidebar info is in its own page. It appears in the sidebar, and the on-page TOC is useful again.

I've touched up the content a little bit to reduce word count, also see the Test plan section below.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Please take a solid look at the Understanding sidebar items section.
Task-oriented docs are generally much better for new users, so I've reworked the titles and description to tell us what each type does immediately, instead of telling us the name of each type first (also helps with SEO).

Let me know if this is correct or if needs improvements, I'll take care of it swiflty.

The rest has only changed a little.

@facebook-github-bot
Copy link
Contributor

Hi @ArtFlag!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@netlify
Copy link

netlify bot commented Dec 9, 2020

✔️ Deploy preview for docusaurus-2 ready!
Built without sensitive environment variables

🔨 Explore the source changes: 51b74f7

🔍 Inspect the deploy logs: https://app.netlify.com/sites/docusaurus-2/deploys/5fd35bc02564a100073ae71c

😎 Browse the preview: https://deploy-preview-3899--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Dec 9, 2020

Size Change: +15 B (0%)

Total Size: 154 kB

ℹ️ View Unchanged
Filename Size Change
website/build/blog/2017/12/14/introducing-docusaurus/index.html 20.7 kB -3 B (0%)
website/build/docs/introduction/index.html 180 B 0 B
website/build/index.html 5.82 kB -1 B
website/build/main.********.js 109 kB +19 B (0%)
website/build/styles.********.css 17.5 kB 0 B

compressed-size-action

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 9, 2020
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ArtFlag ArtFlag changed the title Move sidebar content to its own page docs(v2): Move sidebar content to its own page Dec 10, 2020
website/docs/sidebar.md Outdated Show resolved Hide resolved
@slorber
Copy link
Collaborator

slorber commented Dec 10, 2020

Thanks, that looks like a good idea to me, but will be easier to review if the deploy preview works :)

@slorber slorber added the pr: documentation This PR works on the website or other text documents in the repo. label Dec 10, 2020
@github-actions
Copy link

github-actions bot commented Dec 10, 2020

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 56
🟢 Accessibility 99
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-3899--docusaurus-2.netlify.app/classic/

@slorber
Copy link
Collaborator

slorber commented Dec 10, 2020

@ArtFlag there is now a problem in the deploy preview:

https://deploy-preview-3899--docusaurus-2.netlify.app/classic/docs/sidebar

image

Also, can you comment in this PR the parts of the doc you modified?

As the content is both moved/updated, highlighting the major doc changes would be helpful, but overall it looks good

@ArtFlag
Copy link
Contributor Author

ArtFlag commented Dec 11, 2020

Thanks for fixing the issues!

I've mostly modified the section about the type property.

The rest of the page has seen some modifications but mostly to reduce word count.

For example, for the shorthand note that you mentioned: we used to give the shorthand first and then explain what the "normal" version is.
I flipped it around because we talk about the side item object in this page, so didactically, we should probably show the non-shorthand version first... which actually shows the sidebar item object 😉

No content has been deleted.

@slorber
Copy link
Collaborator

slorber commented Dec 11, 2020

Thanks

Can you fix the git conflict and the admonition mistake so that I can merge this?
(let me know if you need help)

@ArtFlag ArtFlag force-pushed the docs/update-sidebar branch from ca58643 to 51b74f7 Compare December 11, 2020 11:45
@ArtFlag
Copy link
Contributor Author

ArtFlag commented Dec 11, 2020

It should be fine now.

FYI I couldn't push my docs update because pre-commit thought it was an empty commit. Not sure if this update came from my rebase on master, but if there's a way to fix (ignore docs?) that would be nice 👍

@slorber
Copy link
Collaborator

slorber commented Dec 11, 2020

thanks

not sure what you are talking about but that looks fine to merge :)

@slorber slorber merged commit 109a4a7 into facebook:master Dec 11, 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: documentation This PR works on the website or other text documents in the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants