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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve sidebar height when toggling sidebar #4415

Closed
Simek opened this issue Mar 12, 2021 · 26 comments 路 Fixed by #8328
Closed

Preserve sidebar height when toggling sidebar #4415

Simek opened this issue Mar 12, 2021 · 26 comments 路 Fixed by #8328
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. domain: theme Related to the default theme components status: accepting pr This issue has been accepted, and we are looking for community contributors to implement this

Comments

@Simek
Copy link
Contributor

Simek commented Mar 12, 2021

馃悰 Bug Report

Refs: #4311

Currently the left sidebar do not preserve the height on some of short pages (with long sidebars?), when toggling between collapsed and expanded states.

This is causing the visual "jump" of footer each time user interacts with sidebar mode.

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

  1. Navigate to: https://v2.docusaurus.io/docs/markdown-features
  2. Toggle sidebar
  3. (optional) Expand more sidebar groups to increase "jump" height.

EDIT @slorber: note to self: the content is smaller than the sidebar. Don't use the Markdown features page: as of 2022 it is now taller. A better repro page is https://docusaurus.io/tests/docs

Expected behavior

Footer is not "jumping", sidebar height is preserved.

Actual Behavior

Footer is "jumping", sidebar height is not preserved.

Preview

jump

Your Environment

N/A

Reproducible Demo

@Simek Simek added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Mar 12, 2021
@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Oct 30, 2021
@Josh-Cena Josh-Cena added difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. status: accepting pr This issue has been accepted, and we are looking for community contributors to implement this labels Dec 15, 2021
@tapanchudasama
Copy link
Contributor

I would like to pick this up, if anybody has not started any work on this.

@slorber
Copy link
Collaborator

slorber commented Jan 6, 2022

Thanks, nobody is on it, just submit a PR :)

@Josh-Cena Josh-Cena added the domain: theme Related to the default theme components label Apr 10, 2022
@Josh-Cena
Copy link
Collaborator

@lex111 What do you think about this? Would a UX like this make more sense?

Test

I don't know a very robust way to implement it though, and the more I look at it, the more I think a collapse button with persisted vertical position makes sense (see #6717)

@lex111
Copy link
Contributor

lex111 commented Apr 11, 2022

It's natural to expect that expanding the content part will shift of elements below, and vice versa. Therefore I don't see why that's such a big issue.

@yzhe819

This comment was marked as off-topic.

@Josh-Cena

This comment was marked as off-topic.

@Josh-Cena

This comment was marked as off-topic.

@yzhe819

This comment was marked as off-topic.

@slorber slorber changed the title Preserve sidebar height when toggling between modes Layout shift when collapsing the sidebar Aug 9, 2022
@slorber
Copy link
Collaborator

slorber commented Aug 9, 2022

Edit: my comment and discussions above => moved to #7918 (comment) which indeed is a separate issue

@Josh-Cena Josh-Cena changed the title Layout shift when collapsing the sidebar Preserve sidebar height when toggling sidebar Aug 9, 2022
@Josh-Cena
Copy link
Collaborator

Reverted the title change to prevent confusion with #7918, which are two distinct issues (this one is about maintaining sidebar height; that one is about maintaining page scroll position)

@Josh-Cena
Copy link
Collaborator

Here's a reproduction on a "simulated large screen" (via zoom level) in case that helps with understanding:

Test

Note there's no content layout shift. Only the footer shifts.

@slorber
Copy link
Collaborator

slorber commented Aug 9, 2022

This is a much better test case, no need to set any zoom level, just click the sidebar collapse button: https://docusaurus.io/tests/docs

CleanShot 2022-08-09 at 18 33 49

@0916dhkim
Copy link
Contributor

I opened a small PR to solve this issue here #8195

@slorber
Copy link
Collaborator

slorber commented Oct 13, 2022

Note this page looks also weird on a vertical screen, we should fix that too:

https://docusaurus.io/tests/docs

CleanShot 2022-10-13 at 11 52 52

There's even a little layout shift on hydration:

CleanShot 2022-10-13 at 11 54 20

@slorber slorber added difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. and removed difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. labels Oct 13, 2022
@pea-nut-z
Copy link

pea-nut-z commented Oct 13, 2022

A reproduction of (#4415 (comment))

menu_longer_than_content

I believe this only happens when the right content is shorter than the navbar menu. The footer shifts because the additional space needed to fit in an open navbar menu longer than the right content. All the right content is longer than the navbar menu now, so this shouldn't be a problem anymore.

Here is an example where the content is longer than the menu and works just fine. However, it is working also because there is no word wrap issue, as discussed in (#7918)
content_no_shift

The cause for the footer shift should be only the word wrap now. Let me know what you think. Thanks!

@pea-nut-z
Copy link

pea-nut-z commented Oct 13, 2022

Note this page looks also weird on a vertical screen, we should fix that too:

Would you like a CSS detection for the vertical screen and then utilize the same navbar reveal on mobile devices - a button in the top left corner?

@0916dhkim
Copy link
Contributor

Would you like a CSS detection for the vertical screen

So something like @media (min-aspect-ratio: 8/5) ?

@pea-nut-z
Copy link

pea-nut-z commented Oct 14, 2022

Would you like a CSS detection for the vertical screen

So something like @media (min-aspect-ratio: 8/5) ?

Check out this

@slorber
Copy link
Collaborator

slorber commented Oct 14, 2022

This is not a problem of vertical/horizontal/ratio but just having more vh than the sidebar + main content

CleanShot 2022-10-14 at 14 22 46@2x

There are good existing repros to both problems here:

@pea-nut-z
Copy link

pea-nut-z commented Oct 14, 2022

This is not a problem of vertical/horizontal/ratio but just having more vh than the sidebar + main content

Thanks for explaining that. Here is what I did.
min_height-navbar

Move the navbar to its outer container - "main-wrapper" in blue, so it will always take up the white space available, giving it a minimum height instead of depending on the content length.

The result
give_nav_min_height

Is this what you're looking for?

@slorber
Copy link
Collaborator

slorber commented Oct 19, 2022

@pea-nut-z honestly not sure what I want exactly, need to see it live and play with it to feel the UX on different edge cases.

Can you open a PR with this suggestion?

@lex111
Copy link
Contributor

lex111 commented Oct 22, 2022

This is not a problem of vertical/horizontal/ratio but just having more vh than the sidebar + main content

It shouldnt be that way, Ive fixed it erlier, apparently it's broken again #7025

@0916dhkim
Copy link
Contributor

@slorber Would you look at #8328 and let me know what you think?

@slorber
Copy link
Collaborator

slorber commented Nov 24, 2022

This issues' bug is fixed by @0916dhkim in #8328 馃憤 thanks

Opened 2 other related docs layout issues discovered in the process:

@slorber slorber closed this as completed Nov 24, 2022
@cainmagi
Copy link

cainmagi commented Mar 20, 2023

Current version

Docusaurus 2.3.1

Description

This issue appears again because someone made a change from:

.main-wrapper {
  display: block;
}
.main-wrapper docsWrapper_XXXX {
  display: flex;
}

to

.main-wrapper {
  display: flex;
  flex-direction: column;
}
.main-wrapper docsWrapper_XXXX {
  display: flex;
  flex-direction: column;
}

How to reproduce

Just initialize the standard template:

npx create-docusaurus@2.3.1 test-doc classic
cd test-doc
npm start

Then go to this page:

http://localhost:3000/docs/category/tutorial---extras

image

My solution

I made a hot fixture for this issue by modifying the src/css/custom.css:

/* Fix a bug caused by Docusaurus, which makes the sidebar not expand again! */
.docs-wrapper div.main-wrapper {
  flex-direction: row;
}

But I believe that this issue should be solved by modifying the source codes of Docsauraus. Please take a look at the source codes, sir @slorber. Thank you!

@slorber
Copy link
Collaborator

slorber commented Mar 22, 2023

@cainmagi should be fixed for upcoming v2.4 and v3 already:
#8369 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: intermediate Issues that are medium difficulty level, e.g. moderate refactoring with a clear test plan. domain: theme Related to the default theme components status: accepting pr This issue has been accepted, and we are looking for community contributors to implement this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants