-
Notifications
You must be signed in to change notification settings - Fork 5
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
Get Blog Post TOC rendering with numbers #391
Conversation
src/components/BlogPostToc.tsx
Outdated
@@ -138,7 +137,7 @@ export const RenderToc = ({ items }: { items: TocItem[] }) => { | |||
</Typography> | |||
</AccordionSummary> | |||
<AccordionDetails className="accordion-side-padding"> | |||
<ul> | |||
<ol role="list"> |
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 will mark with numbers but also I feel like a more appropriate tag to use semantically.
Visit the preview URL for this PR (updated for commit 5f49f10): https://estuary-marketing--pr391-travjenkins-bug-add-inma2ym6.web.app (expires Thu, 15 Aug 2024 14:15:19 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 76f6b095a0752e5d9c6c890267f9fdc3e392161e |
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.
Lgtm
The main change for SEO is not that we need numbers but that we should display nested headers as well to at least 1 level of depth. |
…accordion by default, replace bullet points with numbers, remove extra margin from accordion summary, and migrate <li> style to classes .tocItem and .isItemSelected
In my commit, I made the following changes:
|
The sections do not have a wrapper, so it's hard to detect if they are a little bit or half into the section or not. That's why it's changing the selected link from TOC when the heading shows up. Instead of:
We have:
|
Okay - that makes sense so we can keep it as is. |
Changes
#278
ol
Tests / Screenshots