Skip to content
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(v2): add TOC to blog posts #3274

Merged
merged 1 commit into from
Aug 17, 2020
Merged

Conversation

amy-lei
Copy link
Contributor

@amy-lei amy-lei commented Aug 12, 2020

Motivation

As discussed in #2717 thought it would be nice to provide a TOC out of the box for blog posts like v1 did.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

image

Start a development server and check out the existing blog posts where a TOC is applicable. The TOC can also be toggled off like with docs by adding hide_table_of_contents to the header.

Related PRs

N/A

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 12, 2020
<div className="container margin-vert--lg">
<div
className="container margin-vert--lg"
style={{maxWidth: 'inherit'}}>
Copy link
Contributor Author

@amy-lei amy-lei Aug 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this was the correct way to handle the layout/style.

I tried to use a 3-6-3 column layout, but because of the max-width property on the container it was very cramped. I didn't know whether it'd be best to override the max-width property, get rid of the container class name, or do something entirely different.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure exactly what to do sorry 🤪 just know that adding the TOC shouldn't reduce as much the main content width

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also avoid inline style objects, use real CSS instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slorber For now, I made the blog TOC use col--2 instead of col--3 unlike the docs' TOC. Although it's narrower, it allows the content widths across all blog pages to be left untouched. I'll keep exploring other approaches and tinkering with the CSS

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Aug 12, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 3af8eea

https://deploy-preview-3274--docusaurus-2.netlify.app

@slorber
Copy link
Collaborator

slorber commented Aug 14, 2020

Hey @amy-lei , that looks nice thanks.

Before your change:

image

After your change:

image

I think it takes too much space. We should increase width of the main blog post content surface, and we should aim to make the design/layout more consistent with what we have for docs:

image

Note sure if it's 100% possible, as blog doesn't have a sidebar, but if we can have the same width for blog/docs TOC with a theme constant that could be nice :)

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that looks like a nice start

  • Avoid inline style, use css modules
  • why modify other pages than the BlogPostPage?
  • move TOC component to separate folder
  • try to sync TOC layout/width between blog/docs
  • option to hide tocs not needed for now

If you really want to provide an option to hide TOC, do it through themeConfig, and provide it for both docs/blog

<div className="container margin-vert--lg">
<div
className="container margin-vert--lg"
style={{maxWidth: 'inherit'}}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure exactly what to do sorry 🤪 just know that adding the TOC shouldn't reduce as much the main content width

website/docs/blog.md Show resolved Hide resolved
<div className="container margin-vert--lg">
<div
className="container margin-vert--lg"
style={{maxWidth: 'inherit'}}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also avoid inline style objects, use real CSS instead

@anshulrgoyal
Copy link
Contributor

I had weird thought if the user wants to swizzle Toc for Docs but not For Blog. May be no one wants to do that but just a thought.

@slorber
Copy link
Collaborator

slorber commented Aug 14, 2020

If the user wants to customize one toc and not the other, he could swizzle the DocPage directly and use his own comp instead of the theme one. If people ask for this we could create TOC, BlogTOC, DocTOC, but that feels overkill for now.

* refactor TOC into own component

* make layout consistent across blog pages
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Aug 17, 2020
@slorber
Copy link
Collaborator

slorber commented Aug 17, 2020

Thanks @amy-lei this is perfect 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants