-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
docs: editorial fixes #6889
docs: editorial fixes #6889
Conversation
✔️ [V2] 🔨 Explore the source changes: a3f8a35 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/62298df8bbc53200096cbdca 😎 Browse the preview: https://deploy-preview-6889--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6889--docusaurus-2.netlify.app/ |
Size Change: +1.07 kB (0%) Total Size: 792 kB
ℹ️ View Unchanged
|
@@ -53,12 +47,19 @@ import TOCInline from '@theme/TOCInline'; | |||
<TOCInline | |||
// Only show h2 and h4 headings | |||
toc={toc.filter((node) => node.level === 2 || node.level === 4)} | |||
minHeadingLevel={2} |
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.
seems weird to use filter + min/max at the same time?
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.
filter
filters out the h3 nodes. But when the h4 nodes are passed into TOCInline
, they are still being ignored by the component if maxHeadingLevel
is not configured to be 4
. I don't think there's much we can do, because no-one knows that a filter
happened on the user-side.
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 see thanks
Motivation
A lot of really minor improvements
Have you read the Contributing Guidelines on pull requests?
Yes