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 ability to hide doc sidebar #2872

Closed
wants to merge 1 commit into from

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Jun 3, 2020

Motivation

This PR allows you to hide the doc sidebar along with increasing content column width.
This is useful when there is a lot of long content on the page and you want to focus on it. This feature can also be actively used on medium screens (e.g. on tablets).

GitBook and GitLab docs have a similar feature in, but there the content column width does not increase, which loses the meaning of this feature, in my view.

As a result, the user has the opportunity to expand the content area width by 300px (this is sidebar width) if they hides the sidebar (see screenshots below).

I would even recommend turning it on by default, because in reality few docs generators have this functionality, this will be our just one more advantage.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Preview.

Before After
image image

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: new feature This PR adds a new API or behavior. label Jun 3, 2020
@lex111 lex111 requested a review from yangshun June 3, 2020 14:50
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 3, 2020
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit 147b304

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

@lex111
Copy link
Contributor Author

lex111 commented Jun 4, 2020

@yangshun what do you think of this feature? Actually, it did not take a lot of code to implement it, but it seems to me that this will be a useful feature for our users. Moreover, that is good from the point of view of UX.

@slorber
Copy link
Collaborator

slorber commented Jun 4, 2020

Hi,

The feature looks a good idea to me, but I'm not a fan of having the collapsed sidebar still being visible (a matter of taste, but I see some apps do that as well).

This feature is mostly useful on medium screens, what about making the sidebar totally disappear, with an ability to toggle/pin it?
I'm thinking about something like:
https://codepen.io/Kristuff/pen/apEEYM

If we keep current designs, some details:

  • During transitions, the sidebar should keep a constant width, otherwise there's FOUC due to text wrapping
  • Not a fan of having the expansion icon so much at the bottom, i'd rather add it to the center of the sidebar with more paddings

@yangshun
Copy link
Contributor

yangshun commented Jun 5, 2020

what do you think of this feature?

I like it, thanks for adding it! It's a nice addition

Not a fan of having the expansion icon so much at the bottom, i'd rather add it to the center of the sidebar with more paddings

Agreed, the current design works but can be made more aesthetically pleasing.

Some ideas:

Visible

Screen Shot 2020-06-05 at 3 21 14 PM

Hidden

Screen Shot 2020-06-05 at 3 26 07 PM

Screen Shot 2020-06-05 at 3 29 59 PM

@slorber slorber marked this pull request as draft June 25, 2020 15:22
@yangshun
Copy link
Contributor

Let's revisit this in future if/when Alexey is back. This was never published and it has a ton of merge conflicts now.

@yangshun yangshun closed this Aug 26, 2020
@yangshun yangshun deleted the lex111/feat-hideable-sidebar branch August 26, 2020 02:02
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.

5 participants