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

Add floating table of contents #2466

Merged
merged 31 commits into from
Jun 15, 2020
Merged

Conversation

rnevius
Copy link
Contributor

@rnevius rnevius commented May 18, 2020

Description

As above. In order to accomplish this, I have completely removed the defunct/mostly unused hugo-material-docs theme, and upgraded Hugo to v0.70.0 for Goldmark and .TableOfContents support.

Updates #2443 (comment)

Motivation and Context

#2443 (comment)

Review Instructions

I'm not in love with the style...but it could just be me. Part of the issue is that Sensu Docs headings are quite long (compared to something like the Grafana docs). Maybe things will look cleaner with shorter headings.

It's also possible that the startLevel/endLevel in the tableOfContents configuration need to be modified.

@rnevius rnevius added the ready for review PR is ready to review label May 18, 2020
@rnevius rnevius requested a review from hillaryfraley May 18, 2020 11:41
@rnevius rnevius self-assigned this May 18, 2020
@cwjohnston cwjohnston temporarily deployed to sensu-docs-feature-navi-bhezum May 18, 2020 11:41 Inactive
Copy link
Contributor

@hillaryfraley hillaryfraley left a comment

Choose a reason for hiding this comment

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

@rnevius I like the look although I see what you mean about longer headings on the page. I think we can reduce or eliminate the number of pages with unwieldy H2 lengths if I rewrite the headings in the API docs. I also noticed a couple pages with zero on-page headings, which makes the empty right-side menu look strange :D I am sure those pages need a heading or two.

One potential issue: it looks like there is something weird happening on pages that contain a highlighted shell code block. Maybe the content in those blocks isn't wrapping?

I'm seeing it on these pages:

It may be happening on other pages too -- I just tried to confirm it's not isolated to a single page.

@hillaryfraley
Copy link
Contributor

Here's a gif that shows the issue I mentioned:

right-side-menu

@rnevius
Copy link
Contributor Author

rnevius commented May 18, 2020

@hillaryfraley nice catch. This bug has been fixed.

@rnevius rnevius temporarily deployed to sensu-docs-feature-navi-bhezum May 18, 2020 14:35 Inactive
@hillaryfraley
Copy link
Contributor

hillaryfraley commented May 18, 2020

@rnevius We are wondering if it's possible to make a few adjustments:

  1. Can the floating TOC have an indicator of the user's place on the page? For example, as they scroll down, can the current section displayed on the page be highlighted in some way in the floating TOC? This is the most important adjustment we'd like to make.

  2. [HOLD] see Expand H2 to display H3 in floating TOC #2474 When a user clicks on one of the H2 headings in the floating TOC, can that H2 heading expand in the floating TOC to list the H3 headings it contains?

  3. Can we evenly adjust the line spacing all H2 and H3 headings? I tested endLevel = 3 in [markup.tableOfContents], and the first H3 heading was closer to the bottom of the H2 heading than it was to the next H3 heading.

@hillaryfraley hillaryfraley added this to the 2020 Docs IA milestone May 20, 2020
@rnevius rnevius temporarily deployed to sensu-docs-feature-navi-bhezum May 21, 2020 13:10 Inactive
@rnevius rnevius temporarily deployed to sensu-docs-feature-navi-bhezum May 21, 2020 13:32 Inactive
@rnevius
Copy link
Contributor Author

rnevius commented May 21, 2020

@hillaryfraley I have updated items (1) and (3) from your previous comment. Item (2) is a little more tricky (not impossible). Can you take a look at the new state of things and let me know if this is something you'd still like to pursue?

@rnevius rnevius requested a review from hillaryfraley May 21, 2020 13:34
@hillaryfraley hillaryfraley temporarily deployed to sensu-docs-feature-navi-bhezum May 21, 2020 13:48 Inactive
@rnevius rnevius temporarily deployed to sensu-docs-feature-navi-hfj593 June 12, 2020 20:18 Inactive
@rnevius
Copy link
Contributor Author

rnevius commented Jun 12, 2020

@hillaryfraley this functionality has been added. As a test, I have added the toc: false page param to /sensu-go/5.20/release-notes

@hillaryfraley hillaryfraley temporarily deployed to sensu-docs-feature-navi-hfj593 June 12, 2020 20:20 Inactive
Copy link
Contributor

@hillaryfraley hillaryfraley left a comment

Choose a reason for hiding this comment

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

@rnevius This is great! Thank you for all of your work.

I won't merge until Monday because I have a little bit of work to do with removing the manual TOCs from the tops of pages. But I think this one is 👍 💯

@hillaryfraley hillaryfraley changed the title Add floating table of contents [Don't Merge Until 6/15] Add floating table of contents Jun 12, 2020
@hillaryfraley hillaryfraley changed the title [Don't Merge Until 6/15] Add floating table of contents Add floating table of contents Jun 12, 2020
Copy link
Contributor

@hillaryfraley hillaryfraley left a comment

Choose a reason for hiding this comment

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

@rnevius I'm sorry to backtrack on this. I just discovered that the offline: false frontmatter attribute doesn't seem to be working now. When I run yarn run server --layoutDir=offline and open a local copy in my browser, I see the Supported Versions page (which should be excluded using offline: false).

edit: I pushed a commit to this branch without thinking. It's changes we'll want anyway (propagating the toc frontmatter attribute to all release notes) but it seems to be interfering with updating this branch against the base.

@rnevius rnevius temporarily deployed to sensu-docs-feature-navi-hfj593 June 13, 2020 06:18 Inactive
@rnevius rnevius temporarily deployed to sensu-docs-feature-navi-hfj593 June 15, 2020 12:36 Inactive
@rnevius
Copy link
Contributor Author

rnevius commented Jun 15, 2020

@hillaryfraley ready for another review. The last couple of issues have been a result of different behavior with the new version of Hugo in this PR. Nice QA.

Copy link
Contributor

@hillaryfraley hillaryfraley left a comment

Choose a reason for hiding this comment

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

@rnevius thank you for resolving everything I found! This looks great. I'm going to merge it today 🎉

@hillaryfraley hillaryfraley merged commit 6c9c3fc into master Jun 15, 2020
@hillaryfraley hillaryfraley deleted the feature/navigation-enhancements branch June 15, 2020 14:14
hillaryfraley added a commit that referenced this pull request Jun 15, 2020
hillaryfraley added a commit that referenced this pull request Jun 15, 2020
@hillaryfraley hillaryfraley restored the feature/navigation-enhancements branch June 15, 2020 17:48
rnevius added a commit that referenced this pull request Jun 16, 2020
hillaryfraley added a commit that referenced this pull request Jun 16, 2020
* Revert "Revert "Add floating table of contents (#2466)" (#2533)"

This reverts commit 940c91b.

* Revert "Revert "Fix katacoda styles (make full width) (#2532)" (#2534)"

This reverts commit 3e67fad.

* Add {{< code >}} to replace {{< highlight >}}

* Add offline `{{< code >}}` shortcode

Co-authored-by: Hillary Fraley <19190208+hillaryfraley@users.noreply.github.com>
@rnevius rnevius deleted the feature/navigation-enhancements branch July 13, 2020 08:58
@hillaryfraley hillaryfraley removed the ready for review PR is ready to review label May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants