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

Left-side menu / TOC jumps around after you click a menu #348

Closed
prdsoftware opened this issue Oct 2, 2020 · 19 comments · May be fixed by #1023
Closed

Left-side menu / TOC jumps around after you click a menu #348

prdsoftware opened this issue Oct 2, 2020 · 19 comments · May be fixed by #1023

Comments

@prdsoftware
Copy link

Is this a bug? When you click on a menu link on the left / TOC section, the vertical scroll bar will jump around and vertically re-position the menu structure, often pushing the link that you've clicked on off-screen. You then need to scroll down to see the link you've click and view any sub-nodes it may have. It's super confusing and leads to a poor user experience.

Is this a bug, or perhaps there is a configuration setting that adjusts this?

Is anyone else experiencing this?

Thanks in advance.

@quickstar
Copy link

Hi
I'm experiencing the same issue. It's very annoying especially if your documentation grows and you have several nested levels. In my opinion the selected chapter should always be visible in the view port of the TOC.

@LisaFC
Copy link
Collaborator

LisaFC commented Oct 2, 2020

Hi folks, do you have an example of this behaviour I can look at? I think my own sites and the examples I've looked at have sufficiently short TOCs that it's not happening.

@LisaFC
Copy link
Collaborator

LisaFC commented Oct 2, 2020

Also what size screen/device are you looking at the site on?

@prdsoftware
Copy link
Author

You can see it on the Kubernetes site here https://kubernetes.io/docs/reference/access-authn-authz/

Click around the sub-levels of this page and you should see the topic you've clicked on be pushed off-screen.

Using a 24in monitor running at 1920x1200

@LisaFC
Copy link
Collaborator

LisaFC commented Nov 19, 2020

Aha! Finally managed to make it happen. Yes, that is weird. Will have a think about how we'd fix that - any ideas welcome.

@narrenfrei
Copy link
Contributor

Here is my suggestion for scroll the menu to active menu item
narrenfrei@09fb143

@narrenfrei
Copy link
Contributor

Added some tweaks for JS scrolling active menu item to center of nav element
https://github.com/narrenfrei/docsy/blob/restructure-sidebar-tree-menu/layouts/partials/sidebar-tree.html

This version have also build in my suggestion for #450 and #449

@narrenfrei
Copy link
Contributor

I've made a first suggestion for #342. But I have noticed, that my JS to scroll doesnt't work within the version with a foldable sidebar menu. I have to take a further look once more.

@prdsoftware
Copy link
Author

Any update on this issue? It looks like it's had some work done on it, but hasn't made it into a release? Would love to see this bug fixed, as it really affects the usability.

@prdsoftware
Copy link
Author

Any news on this one?

@fekete-robert
Copy link
Collaborator

It seems that @narrenfrei's original patch doesn't apply to the current sidebar-tree.html, but with a slight reworking it seems to work:

--- /Users/feketer/Desktop/work/backyards-docs/themes/docsy/layouts/partials/sidebar-tree.html
+++ /Users/feketer/Desktop/work/backyards-docs/themes/docsy-cisco/layouts/partials/sidebar-tree.html
@@ -53,9 +53,9 @@
 <li class="td-sidebar-nav__section-title td-sidebar-nav__section{{ if $withChild }} with-child{{ else }} without-child{{ end }}{{ if $activePath }} active-path{{ end }}{{ if (not (or $show $p.Site.Params.ui.sidebar_menu_foldable )) }} collapse{{ end }}" id="{{ $mid }}-li">
   {{ if (and $p.Site.Params.ui.sidebar_menu_foldable (ge $ulNr 1)) -}}
   <input type="checkbox" id="{{ $mid }}-check"{{ if $activePath}} checked{{ end }}/>
-  <label for="{{ $mid }}-check"><a href="{{ $manualLink }}"{{ if ne $s.LinkTitle $manualLinkTitle }} title="{{ $manualLinkTitle }}"{{ end }}{{ with $s.Params.manualLinkTarget }} target="{{ . }}"{{ if eq . "_blank" }} rel="noopener"{{ end }}{{ end }} class="align-left pl-0 {{ if $active}} active{{ end }} td-sidebar-link{{ if $s.IsPage }} td-sidebar-link__page{{ else }} td-sidebar-link__section{{ end }}{{ if $treeRoot }} tree-root{{ end }}" id="{{ $mid }}">{{ with $s.Params.Icon}}<i class="{{ . }}"></i>{{ end }}<span class="{{ if $active }}td-sidebar-nav-active-item{{ end }}">{{ $s.LinkTitle }}</span></a></label>
+  <label for="{{ $mid }}-check"><a href="{{ $manualLink }}"{{ if ne $s.LinkTitle $manualLinkTitle }} title="{{ $manualLinkTitle }}"{{ end }}{{ with $s.Params.manualLinkTarget }} target="{{ . }}"{{ if eq . "_blank" }} rel="noopener"{{ end }}{{ end }} class="align-left pl-0 {{ if $active}} active{{ end }} td-sidebar-link{{ if $s.IsPage }} td-sidebar-link__page{{ else }} td-sidebar-link__section{{ end }}{{ if $treeRoot }} tree-root{{ end }}" id="{{ $mid }}">{{ with $s.Params.Icon}}<i class="{{ . }}"></i>{{ end }}<span {{ if $active }}class="td-sidebar-nav-active-item" id="td-sidebar-nav-active-item"{{ end }}>{{ $s.LinkTitle }}</span></a></label>
   {{ else -}}
-  <a href="{{ $manualLink }}"{{ if ne $s.LinkTitle $manualLinkTitle }} title="{{ $manualLinkTitle }}"{{ end }}{{ with $s.Params.manualLinkTarget }} target="{{ . }}"{{ if eq . "_blank" }} rel="noopener"{{ end }}{{ end }} class="align-left pl-0{{ if $active}} active{{ end }} td-sidebar-link{{ if $s.IsPage }} td-sidebar-link__page{{ else }} td-sidebar-link__section{{ end }}{{ if $treeRoot }} tree-root{{ end }}" id="{{ $mid }}">{{ with $s.Params.Icon}}<i class="{{ . }}"></i>{{ end }}<span class="{{ if $active }}td-sidebar-nav-active-item{{ end }}">{{ $s.LinkTitle }}</span></a>
+  <a href="{{ $manualLink }}"{{ if ne $s.LinkTitle $manualLinkTitle }} title="{{ $manualLinkTitle }}"{{ end }}{{ with $s.Params.manualLinkTarget }} target="{{ . }}"{{ if eq . "_blank" }} rel="noopener"{{ end }}{{ end }} class="align-left pl-0{{ if $active}} active{{ end }} td-sidebar-link{{ if $s.IsPage }} td-sidebar-link__page{{ else }} td-sidebar-link__section{{ end }}{{ if $treeRoot }} tree-root{{ end }}" id="{{ $mid }}">{{ with $s.Params.Icon}}<i class="{{ . }}"></i>{{ end }}<span {{ if $active }}class="td-sidebar-nav-active-item" id="td-sidebar-nav-active-item"{{ end }}>{{ $s.LinkTitle }}</span></a>
   {{- end }}
   {{- if $withChild }}
   {{- $ulNr := add $ulNr 1 }}
@@ -69,3 +69,8 @@
   {{- end }}
 </li>
 {{- end }}
+
+<script>
+var myElement = document.getElementById('td-sidebar-nav-active-item');
+myElement.scrollIntoView();
+</script>

@LisaFC
Copy link
Collaborator

LisaFC commented May 25, 2022

Excellent! Robert would you mind submitting that fix as a PR and we can see it in preview/review?

@snunez1
Copy link

snunez1 commented May 25, 2022

I wonder if we could take this opportunity to do some general improvements in this area? Issue #342 had some pointers and images of TOC that I thought looked a bit better. The TOC is often a big part of first impressions, since people use it to navigate, and a bit of improvement here could only help.

fekete-robert added a commit to fekete-robert/docsy that referenced this issue May 25, 2022
Useful when the left-side toc is longer than the screen.
Fixes google#348 and google#257

This is an update of the original patch of narrenfrei
@prdsoftware
Copy link
Author

Was this code change ever checked-in to Docsy? Looks like it was close to a fix, but the issue still persists and it's kind of a clunker. :-)

@fekete-robert
Copy link
Collaborator

No, unfortunately it didn't get merged

@LisaFC
Copy link
Collaborator

LisaFC commented Mar 10, 2023

@chalin or @geriom do you want to take a look at Robert's fix and see if it will still work "as is" in our glorious new Bootstrap 5 world?

@huanlin
Copy link
Contributor

huanlin commented Aug 9, 2023

Hey guys,
Just to share how I solve this issue:

First, add a file named body-end.html in the folder /layouts/partials/hooks/.
Then add the following code to body-end.html:

<script > 
  (function() {
    var a = document.querySelector("#td-section-nav");
    addEventListener("beforeunload", function(b) {
        localStorage.setItem("menu.scrollTop", a.scrollTop)
    }), a.scrollTop = localStorage.getItem("menu.scrollTop")
  })()
</script>

It magically solved this issue and I'm happy now.

@fekete-robert
Copy link
Collaborator

@huanlin Worked like a charm, many thanks for the fix! Will you submit a PR so it can get included in the upstream docsy? I believe it would go into the layouts/partials/scripts.html file.

@chalin
Copy link
Collaborator

chalin commented Aug 17, 2023

Thanks @huanlin et al. I'll take a closer look as soon as I can. Closing this issue in favor of #257, since it's a duplicate.

@chalin chalin closed this as completed Aug 17, 2023
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 a pull request may close this issue.

8 participants