feat(theme-classic): new navbar item linking to a sidebar #6139

lmpham1 · 2021-12-21T01:12:58Z

Motivation

This PR resolves #6117

Have you read the Contributing Guidelines on pull requests?

Yes.

Implementations

For the plugin-content-doc, I wrote 2 functions:

getFirstDocIdOfSidebar in plugin-content-doc/src/sidebars/utils.ts, which receives a Sidebar object and returns the docId of that object. If it's a category with autogenerated index, the slug of that index page will be returned instead.
toGlobalSidebar in plugin-content-doc/src/globalData.ts, which map the sidebar name and docId returned from getFirstDocIdOfSidebar function to a GlobalSidebar object, which is then attached to a GlobalVersion object in toGlobalVersion function

For theme-classic/src/theme/NavbarItem, I added a new item called DocSidebarNavbarItem that handle the new navbar item type. It does so by fetching the GlobalVersion object, and retrieve its GlobalSidebar data that already contain a GlobalSidebarLink pointing to its first document

Test Plan

To test my changes, I modified the sidebar and navbar objects in the docusaurus.config.js and sidebar.js files in /website/. These changes are not included in this PR, but I have created a separate test branch on my fork with these changes so that you can test it there.

The detailed changes:

//docusaurus.config.js
navbar: {
  ...
  items: [
    { // test sidebar with a category as first item
      type: 'docSidebar',
      position: 'left',
      id: 'docs',
      label: 'Docs',
    },
    { // test sidebar with a doc as first item
       type: 'docSidebar',
       position: 'left',
       id: 'api',
       label: 'API',
    },
  ...
  ]
}

//sidebar.js
const sidebars = {
  docs: [
    // 'introduction', commented out so that first item would be category with generated-index link
    {
      type: 'category',
      label: 'Getting Started',
      link: {
        type: 'generated-index',
      },
      collapsed: false,
      items: [
        ...
      ],
    },
  api: [ //no changes here, first item is a doc
    'cli',
    'docusaurus-core',
    {
      type: 'autogenerated',
      dirName: 'api',
    },
  ],
}

I also created a demo video with the exact changes above: https://youtu.be/BtJ3306urjI

I also tested category with a doc link, as well as without a category link object, and they also seem to be working.

Related PRs

Might require updating the documentation to include this new feature if this PR is merged.

Feel free to let me know if you want any changes to be made!

…lSidebar() function and getFirstDocIdOfSidebar() function, modified GlobalVersion and toGlobalVersion

…em with 'docSidebar' type, added validation in validateThemeConfig.ts

…gin-content-doc

…where it didn't work with undefined category link object

netlify · 2021-12-21T01:20:52Z

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: bcd01a9

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61d6b982fd35090007725ff4

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

github-actions · 2021-12-21T01:21:52Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	81
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

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

Josh-Cena

Hey, thanks for the amazing work done! The tests you've made have made the review so much easier. The comments are mostly about code style.

Could you also add unit tests for the getFirstDocIdOfSidebar function, and update the existing test snapshots?

Also, we can test this on our website. The API item can be linking to the api sidebar.

I'll be working on this for now

packages/docusaurus-plugin-content-docs/src/globalData.ts

packages/docusaurus-plugin-content-docs/src/sidebars/utils.ts

Josh-Cena · 2021-12-21T11:03:46Z

Our current implementation differs slightly from what @slorber envisioned in that issue. The global data is set as {[sidebarId: string]: {link: string}} because the label can be set on every navbar item and fall back to the sidebar ID, so a plain string would work.

Josh-Cena · 2021-12-22T01:55:03Z

@lmpham1 Thanks! Now it's just docs left. You can just go add docs in this PR instead of creating a new one.

lmpham1 · 2021-12-22T04:39:13Z

@Josh-Cena hey, I just added the documentation for our new navbar item type in website/docs/api/themes/theme-configuration. Feel free to let me know if I need to change anything, or if I should add more examples for category cases.

Josh-Cena

LGTM! Waiting on @slorber since I did change the API design a bit

slorber

Thanks, that looks like a good first version!

There are a few issues that we should fix before merging.

What about dogfooding this feature on our own site? We can replace our docs/api tabs by this new navbar items, this would help to review/validate it works fine

packages/docusaurus-plugin-content-docs/src/globalData.ts

slorber · 2021-12-28T18:26:50Z

packages/docusaurus-plugin-content-docs/src/sidebars/utils.ts

@@ -266,6 +276,45 @@ Available document ids are:
    }
  }

+  function getFirstDocOfSidebar(sidebar: Sidebar):


What about refactor to following signature?

function findFirstLink(sidebar: Sidebar): {label: string, path: string} | undefined

So to make this function return the doc's path, I passed into it a LoadedVersion object to look up the doc's permalink:

function findFirstLink(sidebar: Sidebar, version: LoadedVersion): { label: string; path: string } | undefined

I just refactored the function in my recent commit. Let me know if you think this is a good approach, or if you think it's a bit overkill to pass LoadedVersion into this function.

I think it's overkill. Passing in such a big entity makes this function very hard to reason about and optimize in the future. I prefer the initial implementation: the return type is a bit complicated, but it's lightweight and easy to be piped into other processing.

I reverted that refactor commit. Sorry for this but I do have some opinions on what sidebarUtils should do. It should strictly be a pure function operating on the sidebar and nothing else. Hooking into the version information is always overkill and should be done where it's actually needed.

packages/docusaurus-plugin-content-docs/src/types.ts

packages/docusaurus-plugin-content-docs/src/globalData.ts

packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSidebarNavbarItem.tsx

slorber · 2021-12-28T18:46:04Z

packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSidebarNavbarItem.tsx

+      {...props}
+      className={clsx(props.className, {
+        [activeDocInfimaClassName]:
+          activeDoc?.sidebar && activeDoc.sidebar === sidebarId,


Suggested change

activeDoc?.sidebar && activeDoc.sidebar === sidebarId,

activeDoc?.sidebar === sidebarId,

Should be fine considering sidebarId is mandatory

Wonder if the item would be highlighted when we browse a category index 🤪

packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSidebarNavbarItem.tsx

…barNavbarItem.tsx Co-authored-by: Sébastien Lorber <[email protected]>

…le to look for sidebars in all version

slorber · 2021-12-30T09:37:13Z

packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSidebarNavbarItem.tsx

@@ -15,11 +15,19 @@ import {useDocsPreferredVersion, uniq} from '@docusaurus/theme-common';
 import type {GlobalDataVersion} from '@docusaurus/plugin-content-docs';

 function getSidebarInVersion(versions: GlobalDataVersion[], sidebarId: string) {
-  const allSidebars = versions.flatMap((version) => version.sidebars);
+  const allSidebars = versions.map((version) => version.sidebars);


I'd rather use a flat 1-level array here. You can create an array of sidebar entries [sidebarId,sidebar]

const allSidebars = versions.flatMap((version) => Object.entries(version.sidebars));

This makes the following code simpler:

const sidebarEntry = allSidebars.find((sidebarEntry) => sidebarEntry[0] === sidebarId)

slorber · 2021-12-30T09:38:41Z

packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSidebarNavbarItem.tsx

+      return sidebarItem ? sidebarItem[1] : undefined;
+    }
+    return undefined;
+  })[0];


that does not look like a good algo to me (result might not be at index 0, at least you'd want to "compact" the array and remove undefined values) and this is too complex to read. See my suggestion above

…refactor getSidebarLink in DocSidebarNavbarItem

…malink from within the function

…oc's permalink from within the function" This reverts commit 3192dcb. Signed-off-by: Joshua Chen <[email protected]>

packages/docusaurus-plugin-content-docs/src/globalData.ts

slorber

Looks almost good, just minor changes 👍

slorber · 2022-01-05T18:21:45Z

packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSidebarNavbarItem.tsx

+      exact
+      {...props}
+      className={clsx(props.className, {
+        [activeDocInfimaClassName]: activeDoc?.sidebar === sidebarId,


oops, this does not work with "generated index", but it's not a problem with current PR, see #6268

We can solve this separately

website/docs/api/themes/theme-configuration.md

Co-authored-by: Sébastien Lorber <[email protected]>

slorber · 2022-01-06T10:52:22Z

LGTM, thanks !

lmpham1 added 4 commits December 19, 2021 02:27

<feat>(content-docs): added GlobalSidebar, GlobalSidebarLink, toGloba…

54f1468

…lSidebar() function and getFirstDocIdOfSidebar() function, modified GlobalVersion and toGlobalVersion

<feat>(theme-classic): added DocSidebarNavbarItem as the new NavbarIt…

1f71b22

…em with 'docSidebar' type, added validation in validateThemeConfig.ts

<feat>(theme-classic, content-doc): normalized doc id into URL in plu…

99f52a3

…gin-content-doc

<feat>(content-docs): fixed a bug in getFirstDocIdOfSidebar function …

e75ab58

…where it didn't work with undefined category link object

lmpham1 requested review from Josh-Cena, lex111 and slorber as code owners December 21, 2021 01:12

facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 21, 2021

Josh-Cena changed the title ~~<feat>(theme-classic, content-docs): Link a navbar item to a sidebar's first item~~ feat(theme-classic): new navbar item linking to a sidebar Dec 21, 2021

Josh-Cena added the pr: new feature This PR adds a new API or behavior. label Dec 21, 2021

Josh-Cena requested changes Dec 21, 2021

View reviewed changes

packages/docusaurus-plugin-content-docs/src/globalData.ts Outdated Show resolved Hide resolved

packages/docusaurus-plugin-content-docs/src/globalData.ts Outdated Show resolved Hide resolved

packages/docusaurus-plugin-content-docs/src/sidebars/utils.ts Outdated Show resolved Hide resolved

Josh-Cena added 6 commits December 21, 2021 17:58

Merge branch 'main' into navbar-sidebar-first-doc

7a6813b

Various refactors

4c9015b

id => sidebarId

1b30b5a

Link should always be defined

e41a956

Merge branch 'main' into navbar-sidebar-first-doc

8d79727

Fallback to sidebar ID

3e3c280

Josh-Cena added the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Dec 21, 2021

Added unit tests for getFirstDocOfSidebar

939d0a3

added documentation for the new DocSidebarNavbarItem type

5941f7b

Update theme-configuration.md

8b926c6

Josh-Cena approved these changes Dec 22, 2021

View reviewed changes

Merge branch 'main' into navbar-sidebar-first-doc

00bc1ac

slorber requested changes Dec 28, 2021

View reviewed changes

Update packages/docusaurus-theme-classic/src/theme/NavbarItem/DocSide…

95bf5ea

…barNavbarItem.tsx Co-authored-by: Sébastien Lorber <[email protected]>

changed getSidebarInVersion function in the DocSidebarNavbarItem modu…

04e70b4

…le to look for sidebars in all version

slorber reviewed Dec 30, 2021

View reviewed changes

Josh-Cena removed the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Dec 30, 2021

lmpham1 and others added 6 commits December 30, 2021 17:25

refactor findFirstLink and toGlobalSidebars functions in doc plugin, …

1c7c848

…refactor getSidebarLink in DocSidebarNavbarItem

make findFirstLink receive a LoadedVersion object to find a doc's per…

3192dcb

…malink from within the function

Merge branch 'main' into navbar-sidebar-first-doc

2ae7dfe

Revert "make findFirstLink receive a LoadedVersion object to find a d…

95643c8

…oc's permalink from within the function" This reverts commit 3192dcb. Signed-off-by: Joshua Chen <[email protected]>

fix tests

ae7be3a

Lift type

794ac8c

slorber reviewed Jan 5, 2022

View reviewed changes

packages/docusaurus-plugin-content-docs/src/globalData.ts Outdated Show resolved Hide resolved

packages/docusaurus-plugin-content-docs/src/globalData.ts Outdated Show resolved Hide resolved

slorber reviewed Jan 5, 2022

View reviewed changes

slorber approved these changes Jan 5, 2022

View reviewed changes

Josh-Cena and others added 4 commits January 6, 2022 07:24

Update packages/docusaurus-plugin-content-docs/src/globalData.ts

04c196a

Co-authored-by: Sébastien Lorber <[email protected]>

fixes

ec7dd46

Fix docs

a2ad091

Update utils.ts

bcd01a9

slorber merged commit eade41a into facebook:main Jan 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(theme-classic): new navbar item linking to a sidebar #6139

feat(theme-classic): new navbar item linking to a sidebar #6139

lmpham1 commented Dec 21, 2021

netlify bot commented Dec 21, 2021 •

edited

Loading

github-actions bot commented Dec 21, 2021 •

edited

Loading

Josh-Cena left a comment •

edited

Loading

Josh-Cena commented Dec 21, 2021

Josh-Cena commented Dec 22, 2021

lmpham1 commented Dec 22, 2021

Josh-Cena left a comment

slorber left a comment

slorber Dec 28, 2021

lmpham1 Dec 30, 2021 •

edited

Loading

Josh-Cena Jan 2, 2022 •

edited

Loading

slorber Dec 28, 2021

slorber Dec 28, 2021

slorber Dec 30, 2021

slorber Dec 30, 2021

slorber left a comment

slorber Jan 5, 2022

slorber commented Jan 6, 2022

	activeDoc?.sidebar && activeDoc.sidebar === sidebarId,
	activeDoc?.sidebar === sidebarId,

feat(theme-classic): new navbar item linking to a sidebar #6139

feat(theme-classic): new navbar item linking to a sidebar #6139

Conversation

lmpham1 commented Dec 21, 2021

Motivation

Have you read the Contributing Guidelines on pull requests?

Implementations

Test Plan

Related PRs

netlify bot commented Dec 21, 2021 • edited Loading

github-actions bot commented Dec 21, 2021 • edited Loading

Josh-Cena left a comment • edited Loading

Choose a reason for hiding this comment

Josh-Cena commented Dec 21, 2021

Josh-Cena commented Dec 22, 2021

lmpham1 commented Dec 22, 2021

Josh-Cena left a comment

Choose a reason for hiding this comment

slorber left a comment

Choose a reason for hiding this comment

slorber Dec 28, 2021

Choose a reason for hiding this comment

lmpham1 Dec 30, 2021 • edited Loading

Choose a reason for hiding this comment

Josh-Cena Jan 2, 2022 • edited Loading

Choose a reason for hiding this comment

slorber Dec 28, 2021

Choose a reason for hiding this comment

slorber Dec 28, 2021

Choose a reason for hiding this comment

slorber Dec 30, 2021

Choose a reason for hiding this comment

slorber Dec 30, 2021

Choose a reason for hiding this comment

slorber left a comment

Choose a reason for hiding this comment

slorber Jan 5, 2022

Choose a reason for hiding this comment

slorber commented Jan 6, 2022

netlify bot commented Dec 21, 2021 •

edited

Loading

github-actions bot commented Dec 21, 2021 •

edited

Loading

Josh-Cena left a comment •

edited

Loading

lmpham1 Dec 30, 2021 •

edited

Loading

Josh-Cena Jan 2, 2022 •

edited

Loading