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

Generate ToC with Hugo rather than JavaScript #1851

Merged
merged 5 commits into from
Jun 11, 2024

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented Jun 8, 2024

Thanks to the .RenderShortcodes function added in Hugo 0.117.0, we can include the CS modules and all the headings are included in the generated ToC, with unique IDs. This avoids to do it with JavaScript in the browser.

This should fix the issue in #1841.

This might however leave some duplicate IDs in the code generated from OpenAPI definitions.

Pull Request Checklist

Preview: https://pr1851--matrix-spec-previews.netlify.app

@zecakeh zecakeh requested a review from a team as a code owner June 8, 2024 14:56
Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
@@ -2805,42 +2805,42 @@ operations and run in a resource constrained environment. Like embedded
applications, they are not intended to be fully-fledged communication
systems.

{{< cs-module name="instant_messaging" >}}
Copy link
Member

Choose a reason for hiding this comment

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

hrm. We switched to {{< deliberately (#1205). Switching back seems like it will break things?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, this is ok because the cs-module shortcode is switching to .RenderShortcodes instead of .Content? But why is it necessary?

Copy link
Contributor Author

@zecakeh zecakeh Jun 11, 2024

Choose a reason for hiding this comment

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

Hugo is lazy and uses the markdown renderer to detect headings and construct the headings tree.

.Content returns a page rendered as HTML, and {{< means "don't process the data returned by this shortcode with the renderer", so the renderer in the main page just replaces the shortcode with the module page rendered as HTML without processing anything, and Hugo is only aware of the headings in the main page.

.RenderShortcodes returns the raw markdown, with the shortcodes of the page already replaced, and {{% means "process the data returned by this shortcode with the renderer", so the renderer processes the full mardown of the main page and the module pages as a single document, and Hugo is aware of all the headings in the final page.

@@ -4,3 +4,4 @@
IgnoreDirectoryMissingTrailingSlash: true
DirectoryPath: spec
CheckExternal: false
IgnoreInternalEmptyHash: true
Copy link
Member

Choose a reason for hiding this comment

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

for the record: this stops htmltest complaining about links with href="#"

Copy link
Member

Choose a reason for hiding this comment

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

(Though, as far as I can tell, we've had such links in the ToC for ever, so I don't know why we suddenly need this now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

htmltest only tests the HTML source, the ToC was generated with JavaScript, so htmltest didn't check it.

Copy link
Member

Choose a reason for hiding this comment

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

oh of course

layouts/partials/sidebar-tree.html Outdated Show resolved Hide resolved
layouts/partials/toc.html Outdated Show resolved Hide resolved
layouts/partials/toc.html Outdated Show resolved Hide resolved
@@ -4,3 +4,4 @@
IgnoreDirectoryMissingTrailingSlash: true
DirectoryPath: spec
CheckExternal: false
IgnoreInternalEmptyHash: true
Copy link
Member

Choose a reason for hiding this comment

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

(Though, as far as I can tell, we've had such links in the ToC for ever, so I don't know why we suddenly need this now)

layouts/partials/toc.html Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Jun 11, 2024

Writing it here for want of somewhere better: I wondered if we could avoid cloning the whole of sidebar-tree.html by putting toc.html back in baseof.html. The answer is that we can, with some custom CSS, along the lines of:

baseof.html:

-            {{ partial "sidebar.html" . }}
+            <div class="sidebar-div td-sidebar-nav">
+              {{ partial "sidebar.html" . }}
+              {{ partial "toc.html" . }}
+            </div>

Custom CSS:

.sidebar-div {
    overflow-y: auto;
    position: sticky;
    top: 4rem;
    height: calc(100vh - 5rem);
}

.td-sidebar__inner {
    height: auto !important;
    position: static !important;
}

(In other words, we have to move the height and position from .td-sidebar__inner to a higher-level div.) But frankly, that looks more brittle than just forking sidebar-tree.html.

Signed-off-by: Kévin Commaille <[email protected]>
@zecakeh zecakeh requested a review from richvdh June 11, 2024 20:52
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Thanks!

@richvdh richvdh merged commit 784b898 into matrix-org:main Jun 11, 2024
12 checks passed
@zecakeh zecakeh deleted the hugo-toc branch June 11, 2024 21:41
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.

2 participants