fix(theme): preserve sidebar height on collapse #8328

0916dhkim · 2022-11-12T16:59:42Z

Pre-flight checklist

I have read the Contributing Guidelines on pull requests.
If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Test Plan

2022-11-12.12-05-41.mp4

Test links

Deploy preview: https://deploy-preview-8328--docusaurus-2.netlify.app/tests/docs
Same page on prod: https://docusaurus.io/tests/docs

Related issues/PRs

#4415

netlify · 2022-11-12T17:02:50Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`71300e2`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/636fd0ff64e5d200084c88c6
😎 Deploy Preview	https://deploy-preview-8328--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2022-11-12T17:03:39Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟠 63	🟢 97	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 81	🟢 100	🟢 100	🟢 100	🟢 90	Report

0916dhkim

Summary of changes I made:

0916dhkim · 2022-11-12T17:11:50Z

packages/docusaurus-theme-classic/src/theme/DocRoot/Layout/Sidebar/index.tsx

+        <div
+          className={clsx(
+            styles.sidebarViewport,
+            hiddenSidebar && styles.sidebarViewportHidden,
+          )}>
+          <DocSidebar


Wrap the .sidebar element with a new .sidebarViewport element.

0916dhkim · 2022-11-12T17:13:24Z

packages/docusaurus-theme-classic/src/theme/DocRoot/Layout/Sidebar/styles.module.css

+  .sidebarViewport {
+    top: 0;
+    position: sticky;
+    height: 100%;
+    max-height: 100vh;
+  }


.sidebarViewport is a sticky element that's clamped to screen height. Its width will shrink on collapse.
.sidebarViewport, .sidebar, and .expandButton elements will have the same height (on collapse & expand).

0916dhkim · 2022-11-12T17:14:57Z

packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/styles.module.css

@@ -24,8 +20,6 @@

  .sidebarHidden {
    opacity: 0;
-    height: 0;
-    overflow: hidden;
    visibility: hidden;
  }


.sidebar will not shrink in size on collapse anymore. It will just become invisible. This preserves the height.

thanks, looks better indeed 👍

Wonder if it affects accessibility and keyboard navigation? Is it possible now to navigate with the keyboard (tabs) on these hidden elements?

@slorber That's a good point. I haven't tested keyboard navigation on hidden elements.

0916dhkim · 2022-11-12T17:17:26Z

packages/docusaurus-theme-classic/src/theme/DocRoot/Layout/Sidebar/index.tsx

+            onCollapse={toggleSidebar}
+            isHidden={hiddenSidebar}
+          />
+          {hiddenSidebar && <ExpandButton toggleSidebar={toggleSidebar} />}


Put .expandButton inside .sidebarViewport (outside .sidebar).

lex111

Brilliant solution 👍 , I really shouldn't have hidden the sidebar content when collpasing, that was the cause of this bug.

slorber

Thanks, that looks like a good solution 👍

I noticed that our dogfood sidebar with ads on top/bottom now produce some weird behavior, can you take a look please?

It seems related to the presence of the announcementBar

slorber · 2022-11-24T11:50:26Z

packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/styles.module.css

@@ -24,8 +20,6 @@

  .sidebarHidden {
    opacity: 0;
-    height: 0;
-    overflow: hidden;
    visibility: hidden;
  }


thanks, looks better indeed 👍

Wonder if it affects accessibility and keyboard navigation? Is it possible now to navigate with the keyboard (tabs) on these hidden elements?

slorber · 2022-11-24T11:56:55Z

Hmmm, nevermind, can also reproduce this issue on prod 😅 : https://docusaurus.io/tests/docs

slorber · 2022-11-24T13:37:22Z

This doesn't improve that case:

But that's a separate issue, will see if I can fix it in another PR

slorber · 2022-11-24T13:49:22Z

LGTM thanks 👍

fix: preserve sidebar height on collapse

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

71300e2

facebook-github-bot added the CLA Signed label Nov 12, 2022

0916dhkim commented Nov 12, 2022

View reviewed changes

0916dhkim marked this pull request as ready for review November 12, 2022 17:19

0916dhkim requested a review from slorber as a code owner November 12, 2022 17:19

0916dhkim mentioned this pull request Nov 12, 2022

Preserve sidebar height when toggling sidebar #4415

Closed

lex111 reviewed Nov 13, 2022

View reviewed changes

slorber reviewed Nov 24, 2022

View reviewed changes

slorber added the pr: bug fix label Nov 24, 2022

slorber changed the title ~~fix: preserve sidebar height on collapse~~ fix(theme): preserve sidebar height on collapse Nov 24, 2022

slorber merged commit 1bf0589 into facebook:main Nov 24, 2022

slorber mentioned this pull request Nov 24, 2022

Docs layout issue: content container does not grow to take all the available space #8369

Closed

7 tasks

slorber added the to backport label Jan 26, 2023

slorber added backported and removed to backport labels Jan 27, 2023

slorber mentioned this pull request Oct 19, 2023

chore: v3.0.0-rc.0 release #9418

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(theme): preserve sidebar height on collapse #8328

fix(theme): preserve sidebar height on collapse #8328

0916dhkim commented Nov 12, 2022 •

edited by slorber

Loading

netlify bot commented Nov 12, 2022

github-actions bot commented Nov 12, 2022

0916dhkim left a comment

0916dhkim Nov 12, 2022

0916dhkim Nov 12, 2022

0916dhkim Nov 12, 2022

slorber Nov 24, 2022

0916dhkim Nov 24, 2022

0916dhkim Nov 12, 2022

lex111 left a comment

slorber left a comment

slorber Nov 24, 2022

slorber commented Nov 24, 2022

slorber commented Nov 24, 2022 •

edited

Loading

slorber commented Nov 24, 2022

fix(theme): preserve sidebar height on collapse #8328

fix(theme): preserve sidebar height on collapse #8328

Conversation

0916dhkim commented Nov 12, 2022 • edited by slorber Loading

Pre-flight checklist

Motivation

Test Plan

Test links

Related issues/PRs

netlify bot commented Nov 12, 2022

✅ [V2]

github-actions bot commented Nov 12, 2022

⚡️ Lighthouse report for the deploy preview of this PR

0916dhkim left a comment

Choose a reason for hiding this comment

0916dhkim Nov 12, 2022

Choose a reason for hiding this comment

0916dhkim Nov 12, 2022

Choose a reason for hiding this comment

0916dhkim Nov 12, 2022

Choose a reason for hiding this comment

slorber Nov 24, 2022

Choose a reason for hiding this comment

0916dhkim Nov 24, 2022

Choose a reason for hiding this comment

0916dhkim Nov 12, 2022

Choose a reason for hiding this comment

lex111 left a comment

Choose a reason for hiding this comment

slorber left a comment

Choose a reason for hiding this comment

slorber Nov 24, 2022

Choose a reason for hiding this comment

slorber commented Nov 24, 2022

slorber commented Nov 24, 2022 • edited Loading

slorber commented Nov 24, 2022

0916dhkim commented Nov 12, 2022 •

edited by slorber

Loading

slorber commented Nov 24, 2022 •

edited

Loading