-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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(content-docs): sidebar category linking to document or auto-generated index page #5830
Conversation
Size Change: +11.8 kB (+2%) Total Size: 656 kB
ℹ️ View Unchanged
|
Is it really desired to have autogenerated category index pages? Should we add a config option and default it to It's interesting because I see discussion going around for a year, but seems we never settled down on an API design. Here's what I envision:
I'm skeptical about the autogenerated category index pages. Just like the blog archive, There's also #2537 which concerns routing instead of sidebar generation, but I think we should stay unopinionated and don't do these magic under the hood. Users can use |
😅 this is a draft to have something working locally and explore the problem space and UX possibilities. We'll figure out later how to make it configurable and what the API should look like, it's not in a state worth reviewing for now. |
I personally is quite clueless about how to differentiate between "linked" categories and "non-linked" ones :P Maybe we should just make the "non-linked" ones non-clickable as well so they don't look like links at all? Good luck👍 |
✔️ [V2] 🔨 Explore the source changes: 2daa173 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61a9f0d5bb198c000728c6f8 😎 Browse the preview: https://deploy-preview-5830--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5830--docusaurus-2.netlify.app/ |
…-index" category link is used
packages/docusaurus-plugin-content-docs/src/sidebars/generator.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # website/_dogfooding/docs-tests-sidebars.js # website/sidebars.js
@@ -391,7 +391,7 @@ module.exports = { | |||
}; | |||
``` | |||
|
|||
See it in action in the [Docusaurus Guides pages](../../i18n/i18n-introduction.md). | |||
See it in action in the [Docusaurus Guides pages](/docs/next/category/guides). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...is that really future-proof? Or is it only temporary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅 definitively a shitty temp workaround but better than having the wrong link :p
documented it here: #6041
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should have a hook like useVersionLink
? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, if possible I'd prefer to let users keep using regular markdown links than using hooks in their docs
In general, I'd rather have simplicity in our codebase and userland CSS selectors instead of the simplicity of the HTML structure
I don't feel like the current solution is bad and I'm going to merge it as is for now to not delay this PR even more. If you want to keep improving we can do that in another PR before the next release.
Not sure to understand what you mean here 🤔 |
I just don't like to have additional HTML elements/wrappers where they
are not needed at all, as in this case with regular menu list items.
So for now, I prefer to leave it as it is.
2021-12-03 13:02 GMT+03:00, Sébastien Lorber ***@***.***>:
…> Yes, I understand your concern, it also can be confusing. But on the other
> hand, a "regular" list item doesn't need a wrapper, then why add one for
> it? It is double-edged sword, or simplicity vs consistency, and I'd like
> to have both at the same time, but that can't always be achieved, and at
> times such compromises had to be made.
In general, I'd rather have simplicity in our codebase and userland CSS
selectors instead of the simplicity of the HTML structure
> However if you want, I can do new PR in Infima repo, and add wrapper to
> all list items, also might need to change styles, but is it really
> necessary?
I don't feel like the current solution is bad and I'm going to merge it as
is for now to not delay this PR even more. If you want to keep improving we
can do that in another PR before the next release.
> After all, category index items will be much smaller than regular list
> items in most cases, so it seems kind of redundant to have that wrapper
> for all items.
Not sure to understand what you mean here 🤔
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#5830 (comment)
|
Great, so let's keep it as it is then 👍 Awesome, so it seems this PR is ready, thanks for the help 👍 |
I've suggested an API for configuring e.g. if you have
|
Totally agree @marcusnewton , this is one of a new configuration that I think we should add once we have figured out a good public API For now the code is: // By convention, Docusaurus considers some docs are "indexes":
// - index.md
// - readme.md
// - <folder>/<folder>.md
//
// Those index docs produce a different behavior
// - Slugs do not end with a weird "/index" suffix
// - Auto-generated sidebar categories link to them as intro
export function isConventionalDocIndex(doc: {
source: DocMetadataBase['slug'];
sourceDirName: DocMetadataBase['sourceDirName'];
}): boolean {
// "@site/docs/folder/subFolder/subSubFolder/myDoc.md" => "myDoc"
const docName = path.parse(doc.source).name;
// "folder/subFolder/subSubFolder" => "subSubFolder"
const lastDirName = getLastPathSegment(doc.sourceDirName);
const eligibleDocIndexNames = ['index', 'readme', lastDirName.toLowerCase()];
return eligibleDocIndexNames.includes(docName.toLowerCase());
} I was thinking about exposing a similar function in the docs plugin config options (maybe a simpler interface). Do you really need the granularity of defining those rules on a category level, or it can be more global? I don't like much introducing 3 booleans for that, maybe we should have a way to specify file globbing patterns 🤷♂️ |
@slorber I can definitely work with global config, but not sure if others might want to reintroduce this automatic category linking for certain paths in their tree. |
Glob pattern sounds nice |
This functionality is great. Is there any chance it will be extended to be able to link to a page in src/pages? I have already created landing pages for my category folders with tiles and filter components that I would like to be able to link to directly. Thanks. |
Thanks @KatherineWhan I don't understand what you are trying to do. There's a "link" sidebar item already allowing you to link to any standalone page if you want, and due to our plugin architecture the pages, docs and blogs don't know about each others. If you really want the category link to target a standalone page, you can try to add your tiles and filters to a doc using MDX |
@slorber it would just be |
If the intro is a standalone page, this means the sidebar will disappear and we are leaving the docs plugin. I'd like to understand the motivation to do so: having a mockup of the landing page, what are the tiles and their links, why it's better to hide the docs sidebar in such case etc. Not sure why I can't access the Supabase docs atm (probably my network 🤷♂️ ) but afaik it had tiles on some pages and they are nicely integrated, and still display the docs sidebar: https://supabase.io/docs/ |
Yeah, the sidebar disappearing point makes sense. However it's not the only case that can happen: a regular link item can cause this as well, without displaying the external link icon. Still yes I feel like this feature request is underexplained. @KatherineWhan you can open another feature request issue, detailing your API design and desired effects. |
Hi @Slober and @Josh-Cena Thanks for your responses. Yes, essentially what I was trying to do was We have now had another look at the functionality and decided to add my component to a doc, include the sidebar and link it with I have one other question about the sidebar category linking (that I'm hoping that you can answer that I couldn't find in the help). If you have a link to a doc for category where the items are autogenerated, is there any way to stop the linked doc from displaying twice? It displays once as the file linked to the folder and once wherever it is in the folder of items. For example:
Displays as: Knowledge Base (Overview page displays) Thanks again for your help! :) |
Yes, we have quite a few people complaining about this. We will let you declare a list of subitems to be excluded from the autogenerated slice. |
Breaking changes
Doc index slugs
Docs with the following filename patterns now have better urls/slugs:
docs/dir/subdir/index.md
=> slug becomes/docs/dir/subdir/
instead of/docs/dir/subdir/index
docs/dir/subdir/readme.md
=> slug becomes/docs/dir/subdir/
instead of/docs/dir/subdir/readme
docs/dir/subdir/subdir.md
=> slug becomes/docs/dir/subdir/
instead of/docs/dir/subdir/subdir
You can revert to the former behavior easily with frontmatter:
slug: /dir/subdir/index
Motivation
index.md
) as category introductory docSee also #2643
Not included
The following were considered but are not part of the current PR (may eventually come later):
link: {type: "firstDoc"}
shortcut to automatically link the category to its first child docgenerated-index
link to all categories by default as a plugin optionHave you read the Contributing Guidelines on pull requests?
yes
Test Plan
Unit tests, snapshots and dogfood on website
Test urls:
Docs:
index.md
convention