feat(v2): Sidebar category linking to doc by softwarecurator · Pull Request #3898 · facebook/docusaurus

softwarecurator · 2020-12-09T06:40:11Z

Motivation

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Using the enabled config go_to option, you can make the sidebar on collapsed go to the selected link allowing the user to have one less click to they content they were going to look at; if the developer doesn't want to specify the link the application will run as normally:

module.exports = {
    docs: [
    {
      type: 'category',
      label: 'Docusaurus',
      go_to: 'introduction',
      items: ['introduction', 'design-principles', 'contributing'],
    },
    // ...
  },
};

Preview

NOTE
In packages/docusaurus-theme-classic/src/types.d.ts I changed the proptype to any so I could test the go_to feature, I know adding that back in will be mandatory for this the be merged, is there a way to add the go_to prop to PropSidebarItem?

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.)

github-actions · 2020-12-09T06:47:22Z

Size Change: 0 B

Total Size: 154 kB

ℹ️ View Unchanged

Filename	Size	Change
`website/build/blog/2017/12/14/introducing-docusaurus/index.html`	20.7 kB	0 B
`website/build/docs/introduction/index.html`	180 B	0 B
`website/build/index.html`	5.82 kB	0 B
`website/build/main.********.js`	109 kB	0 B
`website/build/styles.********.css`	17.5 kB	0 B

_{compressed-size-action}

netlify · 2020-12-09T06:48:54Z

❌ Deploy preview for docusaurus-2 failed.
Built without sensitive environment variables

🔨 Explore the source changes: 7765199

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

flossypurse · 2020-12-09T22:26:39Z

👍 We really want to have this feature!

slorber · 2020-12-10T16:39:55Z

Thanks @josephriosIO

I'd also like to see this feature available, as it's quite often requested (#2643)

The current implementation does not seem like it's building:

Maybe you forgot to commit the part allowing the new sidebar attribute (go_to)? Can you fix it so that we can review your PR in the deploy preview more easily?

Also, I'm not totally a fan of the API design and name go_to, it works for this usecase but is not very extensible/future proof.

There's a need to link from the sidebar to:

a document, in the same version
a category index (that we could auto-generate later)
maybe something else I'm not thinking about.

I'm suggesting this API surface instead:

module.exports = {
    docs: [
    {
      type: 'category',
      label: 'Docusaurus',
      // linking to a given doc id (in the current version) 
      link: {type: "doc", id: 'introduction'}
      items: ['introduction', 'design-principles', 'contributing'],
    },
    // ...
  },
};

Later (not in this PR) we could add more types like:

module.exports = {
    docs: [
    {
      type: 'category',
      label: 'Docusaurus',
      // Auto-generated a category index page, displaying the category as a TOC / deep tree
      link: {type: "categoryIndex"}
      items: ['introduction', 'design-principles', 'contributing'],
    },
    // ...
  },
};

module.exports = {
    docs: [
    {
      type: 'category',
      label: 'Docusaurus',
      // Link to the first item in the items list (can be a doc or category)
      // As this may be the most common usecase, we might even add a "link: true" shortcut
      link: {type: "first-item"} 
      items: ['introduction', 'design-principles', 'contributing'],
    },
    // ...
  },
};

Note sure but maybe later we could add new types like:

module.exports = {
    docs: [
    {
     // some global site link
      link: {type: "link", to: "/my/Page"} 
      items: ['introduction', 'design-principles', 'contributing'],
    },
    // ...
  },
};

module.exports = {
    docs: [
    {
      // will link to /myDoc in the currently browsed version (/v1/myDoc, /v2/myDoc etc)
      link: {type: "versionedLink", to: "/myDoc"} 
      items: ['introduction', 'design-principles', 'contributing'],
    },
    // ...
  },
};

Does this API make sense to you?

slorber

See inline comments: the implementation is not reliable enough, and the API surface not extensible enough for what we plan in the future

slorber · 2020-12-10T16:43:36Z

+        if (items.hasOwnProperty(i)) {
+          const childItem = items[i];
+          if (childItem.type === 'link') {
+            if (goTo.includes(childItem.label.toLowerCase())) {


That works but does not really feel like a reliable implementation.
What we want is to link to a doc by using its id, not a doc label.
The label of a doc will be translated on i18n sites, so it's not a reliable way to say "I want this category to link to that doc"

Let me know if you don't find a way to link to doc ids, it might not be easily accessible, I can take a look.

I've been trying to see a way to get link the doc ids but I haven't found a sound a solution any ideas on how I can tackle this?

@slorber I tried something similar in the past and remember it being quite difficult to get the id within this context. I would love to know your approach.

This probably has to be implemented directly in the docs plugin, so that the DocSidebar components directly receive the target link destination instead of receiving the docId and trying to get back the link destination.

Much more data is available in the docs plugin, much less is available through props (it wouldn't be optimized to pass too much props), so we try to only pass the minimum required (here, a link destination is probably enough to handle various scenarios)

the idea is also to move the complexity from the theme to the docs plugin. We want to have multiple themes and don't want to duplicate complicated theme logic in each of them.

slorber · 2020-12-10T16:44:52Z

+    setCollapsed((state) => {
+      return !state;
+    });
+    await sleep(100);


Why do you need that sleep here?

Before setCollapsed was inside a setTimeout, for animation purposes for the categories opening and closing I just removed setTimeout and replacing it with sleep which does the same thing and purpose.

slorber · 2020-12-10T16:47:38Z

+          {...props}>
+          {label}
+        </a>
+      )}


That's looks like quite a lot of code duplication.
Can't we just <Link> in both cases (category is a link, or is not a link)?
You can probably call e.preventDefault() when the category does not link to anywhere and acts like a button (like it used to be)

I agree, I made this change.

slorber · 2020-12-10T16:47:55Z

    return isActive ? false : item.collapsed;
  });
+  const [goToLink, setGoToLink] = useState(false);
+  const [intitalHref, setIntitalHref] = useState('');


I don't really understand why we need these 2 new states to implement this feature.
We are just linking to an URL, why state is needed?

Thank you for the swift response!

Yes I agree I removed goToLink and now looking in renaming initialLink

slorber · 2020-12-10T18:55:40Z

Also something important to consider: the prev/next navigation system.

In such case, where the category links to the 2nd doc (quite unusual)

If we are on the "Contributing" page and press Next, where does this link to? "Installation" or "Configuration"

softwarecurator

Thank you for the swift response!

softwarecurator · 2020-12-10T20:17:41Z

    return isActive ? false : item.collapsed;
  });
+  const [goToLink, setGoToLink] = useState(false);
+  const [intitalHref, setIntitalHref] = useState('');


Thank you for the swift response!

Yes I agree I removed goToLink and now looking in renaming initialLink

softwarecurator · 2020-12-10T20:18:43Z

+        if (items.hasOwnProperty(i)) {
+          const childItem = items[i];
+          if (childItem.type === 'link') {
+            if (goTo.includes(childItem.label.toLowerCase())) {


I've been trying to see a way to get link the doc ids but I haven't found a sound a solution any ideas on how I can tackle this?

softwarecurator · 2020-12-10T20:20:04Z

+    setCollapsed((state) => {
+      return !state;
+    });
+    await sleep(100);


Before setCollapsed was inside a setTimeout, for animation purposes for the categories opening and closing I just removed setTimeout and replacing it with sleep which does the same thing and purpose.

softwarecurator · 2020-12-10T20:20:43Z

+          {...props}>
+          {label}
+        </a>
+      )}


I agree, I made this change.

softwarecurator · 2020-12-10T20:22:00Z

    {
      type: 'category',
      label: 'Getting Started',
+      go_to: 'configuration',


I changed this layout to go_to to link: {type: 'doc', id: 'presets'} to get ready for the next commit that will change from comparing to label to comparing to the id instead.

softwarecurator · 2020-12-10T20:23:06Z


 declare module '@theme/DocSidebar' {
-  import type {PropSidebarItem} from '@docusaurus/plugin-content-docs-types';
+  // import type {PropSidebarItem} from '@docusaurus/plugin-content-docs-types';


This is the issue I am having with deployment, since I'm adding a new field to sidebar.js its falling the test is there somewhere where I can add my new link field?

Not sure what you are talking about

The CI deployment error is currently:

This is an error emitted on purpose by the docs plugin, where you should allow/validate the new link attribute

slorber · 2020-12-15T15:56:03Z

+      setCollapsed((state) => {
+        return !state;
+      });
+      await sleep(100);


here you wait 100ms after setting the state, while before we waited before setting the state
that looks different to me

slorber · 2020-12-15T15:58:17Z

  export type Props = {
    readonly path: string;
-    readonly sidebar: readonly PropSidebarItem[];
+    readonly sidebar: any;


should still be a list of sidebar items

swyxio · 2021-01-05T23:23:36Z

  const sidebarName = permalinkToSidebar[currentDocRoute.path];
  const sidebar = docsSidebars[sidebarName];
-
+  console.log(sidebar);


slorber · 2021-11-04T16:57:42Z

will be replaced by #5830

softwarecurator added 3 commits December 8, 2020 16:17

added go_to config

5eb20d6

This is temp

112ec81

added go_to functionality

6ba1686

softwarecurator requested review from lex111 and slorber as code owners December 9, 2020 06:40

facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 9, 2020

slorber changed the title ~~Go to configured page set in sidebar config~~ feat(v2): Sidebar category linking to doc Dec 10, 2020

slorber mentioned this pull request Dec 10, 2020

Sidebar categories linking to a doc or category index page #2643

Closed

slorber requested changes Dec 10, 2020

View reviewed changes

updates from pull request

7765199

softwarecurator commented Dec 10, 2020

View reviewed changes

slorber reviewed Dec 15, 2020

View reviewed changes

swyxio reviewed Jan 5, 2021

View reviewed changes

ben-qnimble added a commit to qnimble/docusaurus that referenced this pull request Feb 27, 2021

Cleanup code per code review comments on pull request facebook#3898

40dac3d

ben-qnimble mentioned this pull request Feb 27, 2021

Go to selected page #4313

Closed

ben-qnimble added a commit to qnimble/docusaurus that referenced this pull request Mar 1, 2021

Address code review comments from pull request facebook#3898

7525e54

ben-qnimble mentioned this pull request Mar 1, 2021

Finish go to selected paged from category sidebar #4323

Closed

slorber closed this Nov 4, 2021

Uh oh!

Conversation

softwarecurator commented Dec 9, 2020

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

Uh oh!

github-actions Bot commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flossypurse commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slorber Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slorber commented Dec 10, 2020

Uh oh!

softwarecurator left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slorber commented Nov 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

github-actions Bot commented Dec 9, 2020 •

edited

Loading

netlify Bot commented Dec 9, 2020 •

edited

Loading

flossypurse commented Dec 9, 2020 •

edited

Loading

slorber commented Dec 10, 2020 •

edited

Loading

slorber Dec 15, 2020 •

edited

Loading