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

[v2] Option to toggle docs category open by default #2354

Closed
doup opened this issue Mar 4, 2020 · 34 comments · Fixed by #2613 or #2682
Closed

[v2] Option to toggle docs category open by default #2354

doup opened this issue Mar 4, 2020 · 34 comments · Fixed by #2613 or #2682
Assignees
Labels
feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. help wanted Asking for outside help and/or contributions to this particular issue or PR. status: claimed Issue has been claimed by a contributor who plans to work on it.

Comments

@doup
Copy link

doup commented Mar 4, 2020

🚀 Feature

Add a new property on the @docusaurus/plugin-content-docs SidebarItemCategory to set the default state as opened. This should affect only to that one category, no all the categories on the docs.

Have you read the Contributing Guidelines on issues?

Yes.

Motivation

Improve UX.

Pitch

Imagine docs with just two root elements:

  • Introduction
  • Reference v

Where Reference is a category with few other documents inside. This would allow to have this Reference category open by default which would be easier for the user to navigate/explore.

@doup doup added feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. status: needs triage This issue has not been triaged by maintainers labels Mar 4, 2020
@amimas
Copy link

amimas commented Mar 18, 2020

Not sure if this really improves ux.

If I'm a new user to the site and by default the references section is open, I'd probably find it confusing because I need to know the introduction docs first before getting into other details.

The scenario mentioned in the original post is probably an example but it feels more like content organization issue rather than needing a feature from the ssg tool.

Docusaurus also lets you create multiple sidebars that contains different navigation. That could also be an option depending the actual use case.

@doup doup closed this as completed Mar 18, 2020
@yangshun
Copy link
Contributor

yangshun commented Mar 18, 2020 via email

@yangshun yangshun reopened this Mar 18, 2020
@yangshun yangshun added help wanted Asking for outside help and/or contributions to this particular issue or PR. status: claimed Issue has been claimed by a contributor who plans to work on it. and removed status: needs triage This issue has not been triaged by maintainers labels Mar 18, 2020
@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 9, 2020

WIP Approach

These are some rough notes on how I'm thinking of approaching this. Note: they are more meant for me so you can ignore them if you're watching this issue.

  • creating a test site using the suggested Categories: "Introduction", "Reference"
  • see if swizzling the DocSidebar is a an approach we can take and then go from there.

Notes

Hard to tell exactly where the code related to categories being open or not lives, but here's the trail I'm following. The actual component is DocSidebarItem. There is a piece of state that controls the state of whether or not the item is open or closed and that is this:

  const [collapsed, setCollapsed] = useState(item.collapsed)

Now the hard part is walking up the tree to figure out where item.collapsed is being set.

Random thought: why even be able to toggle them?? Shouldn't they always be open? Not sure.

Random thought: what is "swizzling" components?

Okay I got it. Maybe I could swizzle the component DocSidebarItem, figure out how to make it open by default, and then use that to modify the actual code to implement this feature? Might be a good approach.

Resources

Here are the relevant files:

  • packages/docusaurus-theme-classic/src/theme/DocSidebar/index.js
  • packages/docusaurus-plugin-content-docs/src/types.ts

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

I was able to swizzle DocSidebar and make the category open by default.

2020-04-13 14 56 00

Next is how to implement this?

Suggested approach:

Modify the Sidebar object. Currently, it looks like this:

export interface SidebarItemCategory {
  type: 'category';
  label: string;
  items: SidebarItem[];
}

export interface SidebarItemCategoryRaw {
  type: 'category';
  label: string;
  items: SidebarItemRaw[];
}

New proposed interfaces:

export interface SidebarItemCategory {
  type: 'category';
  label: string;
  items: SidebarItem[];
+  collapsed?: boolean;
}

export interface SidebarItemCategoryRaw {
  type: 'category';
  label: string;
  items: SidebarItemRaw[];
+  collapsed?: boolean;
}

This will allow a user to create a sidebars.js that lets them specify if a category may be open like so:

module.exports = {
  someSidebar: [
    {
      type: 'category',
      label: 'Introduction',
      items: ['doc1', 'doc2', 'doc3'],
    },
    {
      type: 'category',
      label: 'Reference',
      items: ['mdx'],
      collapsed: false,
    },
  ],
};

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Todos

  • add failing test to packages/docusaurus-plugin-content-docs/src/__tests__/sidebars.test.ts
  • update interfaces in packages/docusaurus-plugin-content-docs/src/types.ts
  • run tests
  • test change locally by creating a new site
  • Update documentation
  • open PR

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Failing test added:
image

But updating the interfaces did not work as expected...Hmm

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Ah, I had to update a few functions:

  • assertIsCategory
  • normalizeCategoryShorthand

Snapshot written ✅

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Additional suggestion: we should link to the local-third-party-project-testing.md in the CONTRIBUTING.md

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

TIL: you can publish learna to verdaccio like so

yarn lerna publish --registry http://localhost:4873

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Wow...I forgot that before npx we just installed everything globally 😂 I was like "how do I do this if I can change the registry for npx..."

zkat/npx#133 (comment)

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Okay this needs to be documented somewhere:

Testing your changes locally

  1. Set up verdaccio
  2. run yarn lerna publish --registry http://localhost:4873

If you need to test something like a new docusaurus site,

  1. install @docusaurus/init globally with
npm i -g @docusaurus/init --registry http://localhost:4873

This will install from your verdaccio registry
2. create a new site with it

@docusaurus/init init my-website classic

Update: I don't know if that's right...

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 13, 2020

Hmm...now I'm not sure if this is actually the right way to test things locally. Talking out loud here...

I made changes to @docusaurus/plugin-content-docs. This package is used in other packages including @docusaurus/theme-classic.

What I should do is create a new docusaurus site that uses the @docusaurus/preset-classic from my verdaccio registry. Does that mean that I can do this?

  1. npx @docusaurus/init@next init my-website classic
  2. cd my-website
  3. npm install @docusaurus/preset-classic --registry http://localhost:4873

I think that would work because I would be installing the theme-classic from my local registry?

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

I think that would work because I would be installing the theme-classic from my local registry?

To be honest, I'm not sure if that worked...I created a new website with npx then tried installing the local package. Nothing broke, but also confused. I'm not sure if using the collapsed property would have resulted in an error without my changes.

I'm worried that somehow when I run npx it's using my local verdaccio registry.

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Suggestion, the swizzle section of the docs should have an example:

yarn swizzle @docusaurus/theme-classic DocSidebar

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Okay I think I'm getting somewhere...I set up a new site, swizzled the DocSidebar and updated the sidebars.js to use the new property collapsed

module.exports = {
  someSidebar: {
    "Test": [
      {
        "type": "category",
        "label": "Introduction",
        "items": ["doc1"],
        "collapsed": false
      }
    ],
    "Reference": [
      {
        "type": "category",
        "label": "Powering MDX",
        "items": ["doc2"],
        "collapsed": false
      }
    ]
}
}

And I'm getting the error:

Error: Unknown sidebar item keys: collapsed. Item: {"type":"category","label":"Introduction","items":["doc1"],"collapsed":false}
    at assertItem (/Users/jprevite/dev/testing/my-website-swizzled/node_modules/@docusaurus/plugin-content-docs/lib/sidebars.js:34:15)

This is good! Now, if I try what I tried above (using my local package), and restart, the error should go away.

Update

Looks like it did! The functionality isn't fully working, but at least we know the logic related to adding that new key worked!

image

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Continuing to test with my changes...one thing I'm noticing is that the collapsed property is being overwritten somewhere so my changes aren't working as expected. Let's dive deeper and find out why.

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Follow the trail

<DocSidebarItem />

This is where the component is actually rendered. collapsed should be false when I set it to false in the sidebars.js but it's not. Let's move up a level to the parent - <DocSidebar />

<DocSidebar/>

Here, we receive props.docsSidebars.someSidebar (someSidebar is what I called my sidebar in sidebars.js) and the property collapsed is true when it should be false. Let's move up a level to the parent - <DocPage /> (I don't have this yet so I'm going to swizzle it).

<DocPage />

Here, we receive props.docsMetadata.docsSidebars.someSidebar and the property collapsed is true when it should befalse. Let's move up a level to the parent - ???

I'm not seeing a parent...
image

Where the heck are my sidebar.js values being changed?! Time to continue digging...

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Ah-HA! I think it is here that it is happening:

function normalizeCategoryShorthand(
  sidebar: SidebarCategoryShorthandRaw,
): SidebarItemCategoryRaw[] {
  return Object.entries(sidebar).map(([label, items]) => ({
    type: 'category',
    collapsed: true,
    label,
    items,
  }));
}

This means collapsed will always be true - oops!

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

This means collapsed will always be true - oops!

I think I was misunderstanding how it worked. I added another test and it looks like loadSidebars is working as expected:

image

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Hmm...I think I'm looking in the wrong place.

The collapsed property does not correspond to the actual state value. the property does not change (which makes sense).

2020-04-14 16 50 33

However, the property should be determined by the sidebars.js file which is currently not the case.

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 14, 2020

Okay next step is to figure out how to pass down the correct value from the sidebars.js because once we get that, it should work as expected.

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

WOW!

The issue was right in front of my face.

function mutateSidebarCollapsingState(item, path) {
// ...logic
  item.collapsed = !anyChildItemsActive
}

I'll need to find a workaround in this packages/docusaurus-theme-classic/src/theme/DocSidebar/index.js

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Talking out loud here...we have a function mutateSidebarCollapsingState. According to the comments:

Calculate the category collapsing state when a page navigation occurs.
We want to automatically expand the categories which contains the current page

That means when we load the page, expand the categories on the current page.

Cool!

I always find it helpful to use concrete examples. Let's do that to understand what we should implement.

Example 1

I have a site. I have a collapsible sidebar. Here is what I want:

  • if a page is loaded, and that category is collapsible and it's the current page, expand that sidebar item (currently implemented)
  • if i put in my sidebars.js file that I want another category collapsed: false then it should always be open by default (regardless if it's the current page or not)

So what does that look like in pseudologic

// if (isChildItem for curentPage, or the category.collapsed is false) {
// expand
}

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Let's try this out.

Context

// sidebars.js 
module.exports = {
  someSidebar: 
     [
      {
        "type": "category",
        "label": "Introduction",
        "items": ["doc1"],
        "collapsed": true
      },
      {
        "type": "category",
        "label": "Powering MDX",
        "items": ["doc2"],
        "collapsed": false
      }
    ]
}

"Powering MDX" should not be collapsed because it's set to false in sidebars.js

Before

This is what happens before I made any changes. (note: I load the page, "Powering MDX" is collapsed).

2020-04-15 13 27 25

After

I made the change and now it loads and is expanded (not collapsed) like I want!

2020-04-15 14 02 17

And, if I'm on the "Powering MDX", it should be expanded (because it's the current page), which still works as expected.

2020-04-15 14 02 53

Notice how "Introduction" is not expanded by default because collapsed is set to true in sidebars.js

Code changes

function mutateSidebarCollapsingState(item, path) {
+ // only modify item.collapsed if there are child items active and the category collapsed is set to true
+      // eslint-disable-next-line no-param-reassign
+      if (anyChildItemsActive && item.collapsed) {
+        item.collapsed = !anyChildItemsActive;
+      }
}

Next steps

  • Make the changes in actual package (instead of swizzled component)
  • publish to verdaccio
  • verify changes work as expected

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

yarn lerna publish --registry http://localhost:4873 --no-git-tag-version

NOTE: this pushes a new git tag 😅 Which is not desired, you can disable with the flag --no-git-tag-version

https://github.com/lerna/lerna/tree/master/commands/version#--no-git-tag-version

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Now, I am unsure if this works as expected...

  1. npx @docusaurus/init@next init my-website classic
  2. cd my-website
  3. npm install @docusaurus/preset-classic --registry http://localhost:4873

I mean it worked to test changes to a plugin docusaurus-plugin-content-docs but I don't know if it's working to test changes to docusaurus-theme-classic womp womp womp...

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

I tried this:

npm_config_registry=http://localhost:4873/ npx @docusaurus/init@next init doc-plz-work classic

and then

npm install @docusaurus/preset-classic --registry http://localhost:4873

But didn't have any luck...I think the issue is with verdaccio. the packages are showing up correctly on verdaccio
image

However, when I install it, it looks like it's installing from the npm registry (at least the versions make it seem so)
image

Todo

  • figure out how to correctly install form verdaccio

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Going a different route:

npm set registry http://localhost:4873/

Let's see if this works

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Hmm...this is super ugly but it got the right versions:

# set the registry to verdaccio
npm set registry http://localhost:4873/
# install @docusaurus/init globally
npm i -g @docusaurus/init
# run the binary from the location directly
/Users/jprevite/.nvm/versions/node/v10.15.1/lib/node_modules/@docusaurus/init/bin/index.js init test-joe

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Hallelujah!!! IT WORKED!!! (notice how "Powering MDX" is not collapsed!!!!)

image

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 15, 2020

Last thing to do is to update the documentation! 🎉 Then I can open a PR

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 23, 2020

@lex111 seems like we should discuss what went wrong in the implementation and what new approach we should take moving forward

@lex111
Copy link
Contributor

lex111 commented Apr 24, 2020

@jsjoeio in short, this line of code does not need to be changed. I would add an additional condition under which item must be expanded.

item.collapsed = !anyChildItemsActive;

Please see the docs https://v2.docusaurus.io/docs/next/docs#collapsible-categories

I recommend checking the changes directly on the v2 website, rather than creating a new one.

Other than that, you don’t need to use Verdaccio for such changes at all, it only takes more effort from you.
The workflow is described here https://github.com/facebook/docusaurus/blob/master/admin/testing-changes-on-Docusaurus-itself.md#testing
It is very simple.

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 24, 2020

I would add an additional condition under which item must be expanded

Awesome, thanks for the tip!

I recommend checking the changes directly on the v2 website, rather than creating a new one.

Ah, didn't think of that. I'll give that a try!

I'll see if I can work on this next week and submit a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This is not a bug or issue with Docusausus, per se. It is a feature request for the future. help wanted Asking for outside help and/or contributions to this particular issue or PR. status: claimed Issue has been claimed by a contributor who plans to work on it.
Projects
None yet
5 participants