-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
refactor(theme-classic): move all sidebar-related config under themeConfig.docs.sidebar #7277
Conversation
@@ -104,6 +104,10 @@ export type TableOfContents = { | |||
export type ThemeConfig = { | |||
docs: { | |||
versionPersistence: DocsVersionPersistence; |
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.
This API is, AFAIK, undocumented. Is it intended for usage by anyone? I initially thought about docsSidebar
so there's some degree of parallel with navbar
and footer
, but because of the existence of this field, I eventually put it in docs.sidebar
, with one extra level of nesting.
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.
We should likely document versionPersistence
, implemented in #3543
Note some sites may want to minimize the usage of local storage. But it may have been me misinterpreting the GDPR rules and may not be that useful to provide an option for it 🤷♂️
Nesting docs.sidebar
doesn't look like a bad idea anyway, still related to preemptive pluralization 🤪
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-7277--docusaurus-2.netlify.app/ |
Size Change: -4 B (0%) Total Size: 806 kB ℹ️ View Unchanged
|
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.
👍 looks like a nice improvement to do
@@ -104,6 +104,10 @@ export type TableOfContents = { | |||
export type ThemeConfig = { | |||
docs: { | |||
versionPersistence: DocsVersionPersistence; |
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.
We should likely document versionPersistence
, implemented in #3543
Note some sites may want to minimize the usage of local storage. But it may have been me misinterpreting the GDPR rules and may not be that useful to provide an option for it 🤷♂️
Nesting docs.sidebar
doesn't look like a bad idea anyway, still related to preemptive pluralization 🤪
Breaking changes
Nothing complicated to upgrade, and validation errors will tell you what to do:
themeConfig.hideableSidebar
has been moved tothemeConfig.docs.sidebar.hideable
themeConfig.autoCollapseSidebarCategories
has been moved tothemeConfig.docs.sidebar.autoCollapseCategories
So if you previously had:
Change it to:
Pre-flight checklist
Motivation
We are having more and more theme config options that are at the root. To make things more scalable in the future, I propose to namespace them all under
docs.sidebar
.Test Plan
Added two tests to verify that validation schema works.