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

[docs] Update the styling of the TOC #14520

Merged
merged 21 commits into from
Feb 15, 2019
Merged

[docs] Update the styling of the TOC #14520

merged 21 commits into from
Feb 15, 2019

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Feb 13, 2019

This is a good page to compare the before and after:

In addition to the change of styling (updated again since first commit):

  • Smooth scrolling (removed out of consideration for those subject to motion sickness.)
  • Add the current hash to the URL (subject to ongoing discussion) (Merged to get user feedback. Might be removed for v4.)
  • Fix an issue where the current logic selects the next section when you scroll past a heading.
  • Fix an issue where the correct TOC item is not selected when clicking '#' links near the bottom of the page.
  • Fix an issue where the correct TOC item is not selected when clicking TOC links for sections near the bottom of the page.
  • Remove an unneeded call to findActiveIndex() in componentDidMount().
  • Fix a duplicate id (font-awesome) on http://localhost:3000/style/icons.

@mbrookes
Copy link
Member Author

This started as a quick restyle of the TOC, and escalated from there!

@mbrookes mbrookes changed the title Docs toc styling [docs] Update the styling of the TOC Feb 13, 2019
@mbrookes mbrookes added the docs Improvements or additions to the documentation label Feb 13, 2019
@eps1lon

This comment has been minimized.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 13, 2019

I'm trying to find examples of great documentation. Few have a TOCs: developers.google.com, getbootstrap.com, docker.com, most don't have it. After looking at them, I think that we should keep the current logic:

  • The TOCs collapse creates a distraction when reading the documentation.
  • The TOCs collapse prevents to see all the subsection up-front.
  • I don't have any strong case against the hash change based on the scroll. But given none of the documentation I could benchmark do it, I would rather not do it either (maybe I have haven't looked at enough documentation 🤔). Just so people have a consistent behavior between the different documentation they have to browse when working on one project.

One thing I think that we should work on regarding the TOCs: i18n support, it's quite a broken right now in Chinese ✌️.

@mbrookes

This comment has been minimized.

@eps1lon

This comment has been minimized.

@mbrookes
Copy link
Member Author

none of the documentation I could benchmark do it

Here's one you might be familiar with! 🤣
https://material.io/design/components/app-bars-bottom.html

Doing this means that when someone is referencing the documentation, the URL pinpoints the specific section. (If anything, it bugs me when there isn't a specific enough id. Having to say "go to this URL, then scroll down to XXX is a PITA for everyone.)

(Yes, we have the '#' icon, but it's easy to miss, or to forget to click before copying the URL.)

@mbrookes
Copy link
Member Author

mbrookes commented Feb 14, 2019

Maybe add numbers to the indentation to make the distinction between section and subsection clearer?

At one point I had ' - ' in front of subsection titles, but because of wrapped headings, it looked odd, so I dropped it (although there is almost certainly a CSS solution to that which preserves the indentation for subsequent lines).

Perhaps rather than making the currently selected section or sub section bold, we reserve it for top level sections. The highlight should be enough by itself to show the currently selected section, so long as contrast doesn't suffer without the emboldening.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 14, 2019

Here's one you might be familiar with! 🤣

@mbrookes I shouldn't have said "none" :). Then maybe it's just me spending too much time on their documentation, I dislike the way it's structured. They have disabled the headlines click to get the anchor. Yeah, I think that we should use this documentation as an example of what we shouldn't do 🙆‍♂️. I'm curious on the ratio % count of docs website using manual anchor links vs automatic. As somebody often linking documentation anchor, I personally prefer "being in control".

Doing this means that when someone is referencing the documentation

Then why so few documentation websites use this approach? I think that it's because you don't know if a person is going to link the whole page or a subsection, you let him choose. Most of the time, they link the whole page, so you optimize for the majority. It's also an incentive for us to split large documentation pages.

I would need to manually click through those to find the part I'm interested in.

@eps1lon 💯


IMHO we shouldn't try to put too many thoughts into how the TOCs work. I think that a lot of documentation, older & more used than our have spent a lot of time on the problem, I would just copy what the majority do. "The majority is right", I think that it applies in this case. It's what I have tried to do when implementing the TOCs.

@oliviertassinari

This comment has been minimized.

@eps1lon
Copy link
Member

eps1lon commented Feb 14, 2019

Doing this means that when someone is referencing the documentation

Then why so few documentation websites use this approach?

To be fair: This is probably skewed because it's the default behavior of the docs software.

I think having the anchor automatically update is a nice feature. It doesn't require knowledge about position of anchor links or if one even exists. I would even prefer if the anchor links where visible by default.

@mbrookes

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants