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

Fix the sidebar in recent versions of Hugo #1551

Merged
merged 3 commits into from
Jun 27, 2023

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented May 30, 2023

Basically what I did is:

  1. Replace layout/partials/sidebar-tree.html with the one from the matrix-org/docsy fork.
  2. Add back the TOC div.
  3. Adapt the CSS and config to look like before.

Tested in Firefox and Brave with Hugo 0.111.3.

Fixes #1544.

Signed-off-by: Kévin Commaille zecakeh@tedomum.fr

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

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh requested a review from a team as a code owner May 30, 2023 17:03
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@richvdh
Copy link
Member

richvdh commented Jun 9, 2023

For the record, the diff to the version of the file in the docsy theme is:

--- themes/docsy/layouts/partials/sidebar-tree.html	2022-11-08 18:20:50.043218973 +0000
+++ layouts/partials/sidebar-tree.html	2023-06-09 14:31:15.513433423 +0100
@@ -1,3 +1,12 @@
+{{/*
+
+  A version of the sidebar-tree.html partial in Docsy, with a few small
+  modifications:
+
+  * include `div#toc` for the ToC
+
+*/}}
+
 {{/* We cache this partial for bigger sites and set the active class client side. */}}
 {{ $sidebarCacheLimit := cond (isset .Site.Params.ui "sidebar_cache_limit") .Site.Params.ui.sidebar_cache_limit 2000 -}}
 {{ $shouldDelayActive := ge (len .Site.Pages) $sidebarCacheLimit -}}
@@ -31,6 +40,8 @@
     <ul class="td-sidebar-nav__section pr-md-3 ul-{{ $ulNr }}">
       {{ template "section-tree-nav-section" (dict "page" . "section" $navRoot "shouldDelayActive" $shouldDelayActive "sidebarMenuTruncate" $sidebarMenuTruncate "ulNr" $ulNr "ulShow" (add $ulShow 1)) }}
     </ul>
+    <hr/>
+    <div id="toc"></div>
   </nav>
 </div>
 {{ define "section-tree-nav-section" -}}
@@ -68,4 +79,4 @@
   </ul>
   {{- end }}
 </li>
-{{- end }}
\ No newline at end of file
+{{- end }}

... which is certainly a lot simpler than the old diff!


It's very unclear to me where the current version came from. The closest I can find is ffb95959 - which does correspond to the point it was forked (55eae7b#diff-936974843f4ebd4ed902f880848d8640095e1506bb3d6c42bd04114a6746e14a). The diff (with whitespace excluded) was:

--- themes/docsy/layouts/partials/sidebar-tree.html	2023-06-09 14:37:39.807314942 +0100
+++ layouts/partials/sidebar-tree.html	2023-06-09 14:32:52.681686549 +0100
@@ -1,13 +1,17 @@
+{{/*
+
+  A version of the sidebar-tree.html partial in Docsy, with a few small
+  modifications:
+
+  * include `div#toc` for the ToC
+  * start the sidebar at the root (homepage) since for us that is the Matrix
+  overview page
+
+*/}}
+
 {{/* We cache this partial for bigger sites and set the active class client side. */}}
 {{ $shouldDelayActive := ge (len .Site.Pages) 2000 }}
 <div id="td-sidebar-menu" class="td-sidebar__inner{{ if $shouldDelayActive }} d-none{{ end }}">
-  {{ if not .Site.Params.ui.sidebar_search_disable }}
-  <form class="td-sidebar__search d-flex align-items-center">
-    {{ partial "search-input.html" . }}
-    <button class="btn btn-link td-sidebar__toggle d-md-none p-0 ml-3 fas fa-bars" type="button" data-toggle="collapse" data-target="#td-section-nav" aria-controls="td-docs-nav" aria-expanded="false" aria-label="Toggle section navigation">
-    </button>
-  </form>
-  {{ else }}
   <div id="content-mobile">
   <form class="td-sidebar__search d-flex align-items-center">
     {{ partial "search-input.html" . }}
@@ -16,44 +20,42 @@
   </form>
   </div>
   <div id="content-desktop"></div>
-  {{ end }}
   <nav class="collapse td-sidebar-nav" id="td-section-nav">
-    {{ if  (gt (len .Site.Home.Translations) 0) }}
-    <div class="nav-item dropdown d-block d-lg-none">
-      {{ partial "navbar-lang-selector.html" . }}
-    </div>
-    {{ end }}
-    {{ $navRoot := cond (and (ne .Params.toc_root true) (eq .Site.Home.Type "docs")) .Site.Home .FirstSection }}
-    {{ template "section-tree-nav-section" (dict "page" . "section" $navRoot "delayActive" $shouldDelayActive)  }}
+    {{ template "section-tree-nav-section" (dict "page" . "section" .Site.Home.CurrentSection "delayActive" $shouldDelayActive "indent" 0)  }}
+    <hr/>
+    <div id = "toc"></div>
   </nav>
 </div>
+
 {{ define "section-tree-nav-section" }}
 {{ $s := .section }}
 {{ $p := .page }}
 {{ $shouldDelayActive := .delayActive }}
-{{ $active := eq $p.CurrentSection $s }}
-{{ $show := or (and (not $p.Site.Params.ui.sidebar_menu_compact) ($p.IsAncestor $s)) ($p.IsDescendant $s) }}
+    {{ $indent := .indent }}
+    {{ $active := eq $p.RelPermalink $s.RelPermalink }}
+    {{ $show := or ($p.IsAncestor $s) ($p.IsDescendant $s) }}
 {{ $sid := $s.RelPermalink | anchorize }}
 <ul class="td-sidebar-nav__section pr-md-3">
   <li class="td-sidebar-nav__section-title">
-    <a  href="{{ $s.RelPermalink }}" class="align-left pl-0 pr-2{{ if not $show }} collapsed{{ end }}{{ if $active}} active{{ end }} td-sidebar-link td-sidebar-link__section">{{ $s.LinkTitle }}</a>
+    <a  href="{{ $s.RelPermalink }}" class="align-left pl-0 pr-2{{ if not $show }} collapsed{{ end }}{{ if $active}} active{{ end }} td-sidebar-link td-sidebar-link__section indent-{{$indent}}">{{ $s.LinkTitle }}</a>
-  </li>
-  <ul>
-    <li class="collapse {{ if $show }}show{{ end }}" id="{{ $sid }}">
       {{ $pages := where (union $s.Pages $s.Sections).ByWeight ".Params.toc_hide" "!=" true }}
       {{ $pages := $pages | first 50 }}
+    {{ if gt (len $pages) 0 }}
+    <ul>
       {{ range $pages }}
-      {{ if (not (and (eq $s $p.Site.Home) (eq .Params.toc_root true))) }}
         {{ if .IsPage }}
           {{ $mid := printf "m-%s" (.RelPermalink | anchorize) }}
           {{ $active := eq . $p }}
-          <a class="td-sidebar-link td-sidebar-link__page {{ if and (not $shouldDelayActive) $active }} active{{ end }}" id="{{ $mid }}" href="{{ .RelPermalink }}">{{ .LinkTitle }}</a>
+      <li class="collapse {{ if $show }}show{{ end }}" id="{{ $sid }}">
+        <a class="td-sidebar-link td-sidebar-link__page {{ if and (not $shouldDelayActive) $active }} active{{ end }} indent-{{add $indent 1}}" id="{{ $mid }}" href="{{ .RelPermalink }}">{{ .LinkTitle }}</a>
+      </li>
         {{ else }}
-          {{ template "section-tree-nav-section" (dict "page" $p "section" .) }}
+                {{ $indent := add $indent 1 }}
+                {{ template "section-tree-nav-section" (dict "page" $p "section" . "indent" $indent) }}
         {{ end }}
       {{ end }}
+    </ul>
       {{ end }}
     </li>
   </ul>
-</ul>
 {{ end }}

That diff appears to do several things:

  • disable the search bar (now covered by config.toml)
  • remove the "translations" dropdown (there are no translations anyway)
  • hardcode the root of the nav tree to .Site.Home.CurrentSection (seems to be redundant?)
  • changes the algorithm for whether we show an entry ($show)
  • changes the nesting of <li> and <ul> in a way I can't quite follow
  • adds indent-N CSS classes to entries (now handled by upstream via $ulNr/ul-N classes)

@richvdh
Copy link
Member

richvdh commented Jun 9, 2023

@zecakeh One difference I note with the updated version is that the whole menu is expanded. For example, if I go to https://pr1551--matrix-spec-previews.netlify.app/client-server-api/, the "Room Versions" section is expanded (compare https://spec.matrix.org/v1.7/client-server-api/).

Maybe some of the options at https://www.docsy.dev/docs/adding-content/navigation/#section-menu-options could help here?

@richvdh
Copy link
Member

richvdh commented Jun 9, 2023

Another thought: since the only change to upstream is now the addition of the toc element, perhaps we can also get rid of that and instead append the toc to td-section-nav programmatically in the javascript that builds the toc (https://github.com/matrix-org/matrix-spec/blob/main/static/js/toc.js#L115)?

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.

As ever, thanks for your work - this is very helpful and much cleaner than what went before. A couple of thoughts above.

@zecakeh
Copy link
Contributor Author

zecakeh commented Jun 9, 2023

@zecakeh One difference I note with the updated version is that the whole menu is expanded. For example, if I go to https://pr1551--matrix-spec-previews.netlify.app/client-server-api/, the "Room Versions" section is expanded (compare https://spec.matrix.org/v1.7/client-server-api/).

Maybe some of the options at https://www.docsy.dev/docs/adding-content/navigation/#section-menu-options could help here?

Right I didn't notice that. Looking at the options we only have the choice of showing the room versions list only when the "Room Versions" page is selected. We cannot choose to show them only on the overview.

Another option is to make the section collapsible/foldable, so at least users don't need to reload the page to select a specific room version.

Otherwise it looks like we will need to change the sidebar partial some more.

@richvdh
Copy link
Member

richvdh commented Jun 9, 2023

Right I didn't notice that. Looking at the options we only have the choice of showing the room versions list only when the "Room Versions" page is selected.

I think that would be fine, to be honest.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh
Copy link
Contributor Author

zecakeh commented Jun 9, 2023

Alright, now it uses only JS to add the TOC to the sidebar.

@richvdh
Copy link
Member

richvdh commented Jun 27, 2023

(I'd forgotten about this. @zecakeh, please do feel free to ping if you see a PR getting ignored)

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.

LGTM!

@richvdh richvdh merged commit a6eb381 into matrix-org:main Jun 27, 2023
10 checks passed
@zecakeh zecakeh deleted the fix-sidebar branch June 28, 2023 07:49
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.

Table of contents disappears on modern hugo versions
2 participants