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

feat(v2): support external links & linking to docs from other sidebars #1052

Merged

Conversation

SleepWalker
Copy link
Contributor

@SleepWalker SleepWalker commented Oct 20, 2018

This PR implements #827, #868 (v1 PR pending)

Motivation

This PR will improve sidebar to support:

  • new categories array syntax like in fix: change subcategory format #1026 (but I've changed ids to items)
  • links to docs from other sidebars
  • links to other docusaurus non-docs pages
  • links to external resources
  • possibility to easily add new sidebar items types with any custom options

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

I have added some examples in sidebar.json. It looks like this:

{
  "docs": {
    "Category 1": [
      {
        "type": "link",
        "label": "Github",
        "href": "http://github.com"
      },
      {
        "type": "category",
        "label": "Subcategory 1",
        "items": [
          "foo/bar",
          "foo/baz"
        ]
      },
      {
        "type": "category",
        "label": "Subcategory 2",
        "items": [
          {
            "type": "ref",
            "id": "intro"
          },
          "hello",
          {
            "type": "link",
            "label": "Google",
            "href": "http://google.com"
          }
        ]
      }
    ],
    "Category 2": ["code"]
  },
  "test": {
    "Test Ref links": [
      {
        "type": "doc",
        "id": "intro"
      },
      {
        "type": "ref",
        "id": "code"
      }
    ]
  }
}

The type field can be one of the following:

  • doc — low level name for simple link to doc. Should contain id field
  • ref — link to some docId, but without binding this doc to current sidebar. Should contain id field
  • link — the type used to render any link in sidebar (doc and ref are converted to this type too). Should contain href and label fields
  • category — category :) should contain label and items fields (I think this should be a better name, because we will handle not only ids in this array)

I'm normalizing sidebars to this low-level format immediately after reading them (including versioned sidebars) in v2/lib/load/docs/sidebars.js. During normalization I'm checking, that all the items contain only allowed fields and that we won't have sub-sub-categories.

Normalized structure is much more predictable and reliable, which allow us to simplify logic (it won't be needed to test for each type of possible sidebar syntaxes in each function) during docs ordering and during sidebar rendering on client side. In future we can even more simplify this by implementing visitor pattern, like in e.g. babel.

Docs Paginator behavior

I've decided to ignore ref and link items, when building data for docs paginator. If you send user to ref doc, the sidebar will be changed. If you send user to link it may even leave our site. I think that's not expected behavior for the user.

Unit tests

I've added one new test to test for ref and link processing in order.js. All other cases seems to be covered with older tests after I've fixed them

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 20, 2018
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Oct 20, 2018

Deploy preview for docusaurus-preview ready!

Built with commit 4f0b188

https://deploy-preview-1052--docusaurus-preview.netlify.com

Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Kinda happy also by the fact that you are able to implement it in v2 easily @SleepWalker

We need to document this in the future since there is a lot of things now, including 'ref' means including it in sidebar but not including it at next/prev.

The change to ids to items seems to make sense since we allow href linking other than id

https://deploy-preview-1052--munseo-preview.netlify.com/

@endiliey endiliey changed the title Improve sidebar to support external links and linking to docs from other sidebars feat(v2): support external links & linking to docs from other sidebars Oct 21, 2018
@SleepWalker
Copy link
Contributor Author

@yangshun I have more questions:

  • Should I separate v1 implementation or it can be submitted in this PR?
  • What about ids field in v1? Will we leave it without renaming? Or, probably support both ids and items + may be warning?

@yangshun
Copy link
Contributor

@SleepWalker I'll get back to you on this soon. We're currently getting ready for a demo and will be holding off on merging v2 PRs these few days.

@endiliey
Copy link
Contributor

Let's leave v1 implementation in another PR. Small changes is easier to review.

@SleepWalker can you resolve the conflicts ?

@SleepWalker SleepWalker force-pushed the 827-868-v2-sidebar-new-link-types branch from 0966b72 to 9ed899c Compare October 24, 2018 04:14
@SleepWalker
Copy link
Contributor Author

@endiliey yep, they are resolved

@endiliey
Copy link
Contributor

Shall let @yangshun press the merge button

@yangshun yangshun merged commit a2d3f26 into facebook:master Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants