-
Notifications
You must be signed in to change notification settings - Fork 1
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: add table of content on the home page, remove read docs link (r… #141
Conversation
…ycomb-docs into add-table-content
Visit the preview URL for this PR (updated for commit 3797b0d): https://ccv-honeycomb-docs--pr141-add-table-content-4xd47dkf.web.app (expires Wed, 07 Aug 2024 20:14:42 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: d403a110fede5b0308678a87537bf61d0597aef6 |
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 great! Let's pause on merging it in though - we can time it with our 3.4.1 release!
Co-authored-by: Robert Gemma <[email protected]>
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.
Just a little more styling cleanup for the table of contents! Remember, you can always look at the Firebase preview link to make sure it looks how it should
src/pages/styles.module.css
Outdated
/* ... The other nesting */ | ||
} | ||
|
||
.TOC > ul > li { | ||
margin-bottom: 1.5vh; | ||
font-size: large; | ||
} | ||
|
||
.TOC > ul > li > ul { | ||
text-align: center; | ||
list-style: none; | ||
padding: 0; | ||
margin: 0; | ||
font-size: smaller; | ||
} |
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.
Make sure you add these elements to the .TOC class! So it will look something like:
.TOC {
/* styles */
> ul {
/* styles */
> li {
/* styles */
> ul {
/* styles */
}
}
}
}
src/pages/styles.module.css
Outdated
margin: 1.5vh 0; | ||
padding: 0; | ||
|
||
ul { |
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 still needs the >
. It tells css to only style the immediate child of the .TOC class. Using it without the >
will change the styling of any unordered list inside the .TOC
class
ul { | |
> ul { |
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.
It looks like the issue has to do with the name of the class? If you take a look at the inspector in the preview link you can see more than just .TOC
get's added to the class.
Try to see if you can figure out why that's happening, if it's not obvious then it may be best to re-write the table of contents as a JSX component rather than MDX
table_of_content.mdx
Outdated
id: table_of_content | ||
slug: /tableofcontent | ||
title: Table of Content |
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.
Can you rename these to be plural, e.g. "Table of Contents"
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.
What do you think about splitting the table of contents into two rows? I think that may look nice but I'm not sure what the best place to split things is, let me know what you think!
table_of_contents.mdx
Outdated
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 believe this can be deleted now, correct?
Co-authored-by: Robert Gemma <[email protected]>
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.
Love it! Thanks for sticking through all the little style changes 🚀
No problem :) Should I merge this version then? |
Yup! LGTM! |
table_of_contents.mdx
containing a list of table of content items with link to the relative pagessrc/pages/index.js
underHome
functionsrc/pages/styles.module.css
, styling applied to center the table of contents itemsread the docs
button as table of content links will take users to the same location