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

Wrong syntax in sidebar menu tree #450

Closed
narrenfrei opened this issue Feb 16, 2021 · 12 comments
Closed

Wrong syntax in sidebar menu tree #450

narrenfrei opened this issue Feb 16, 2021 · 12 comments

Comments

@narrenfrei
Copy link
Contributor

The sidebar menu tree generates not a correct syntax for nested lists. A nested UL has to start within a LI item.
see: https://developer.mozilla.org/en-US/docs/Learn/HTML/Introduction_to_HTML/HTML_text_fundamentals#nesting_lists

Also there are more ULs generated then needed and some ILs are missing.

This is the (shortend) current code of https://www.docsy.dev/docs/

<nav class="collapse td-sidebar-nav" id="td-section-nav">
  <ul class="td-sidebar-nav__section pr-md-3">
    <li class="td-sidebar-nav__section-title">
      <a href="/docs/" class="align-left pl-0 pr-2 active td-sidebar-link td-sidebar-link__section">Documentation</a>
    </li>
    <ul>
      <li class="collapse show" id="docs">
        <ul class="td-sidebar-nav__section pr-md-3">
          <li class="td-sidebar-nav__section-title">
            <a href="/docs/getting-started/" class="align-left pl-0 pr-2 td-sidebar-link td-sidebar-link__section">Getting Started</a>
          </li>
          <ul>
            <li class="collapse show" id="docsgetting-started">
            </li>
          </ul>
        </ul>
        <ul class="td-sidebar-nav__section pr-md-3">
          <li class="td-sidebar-nav__section-title">
            <a href="/docs/adding-content/" class="align-left pl-0 pr-2 td-sidebar-link td-sidebar-link__section">Content and Customization</a>
          </li>
          <ul>
            <li class="collapse show" id="docsadding-content">
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentcontent" href="/docs/adding-content/content/">Adding Content</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentlookandfeel" href="/docs/adding-content/lookandfeel/">Look and Feel</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentnavigation" href="/docs/adding-content/navigation/">Navigation and Search</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentversioning" href="/docs/adding-content/versioning/">Doc Versioning</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentshortcodes" href="/docs/adding-content/shortcodes/">Docsy Shortcodes</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contenticonsimages" href="/docs/adding-content/iconsimages/">Logos and Images</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentprint" href="/docs/adding-content/print/">Print Support</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentfeedback" href="/docs/adding-content/feedback/">Analytics and User Feedback</a>
              <a class="td-sidebar-link td-sidebar-link__page " id="m-docsadding-contentrepository-links" href="/docs/adding-content/repository-links/">Repository Links</a>
            </li>
          </ul>
        </ul>
        <ul class="td-sidebar-nav__section pr-md-3">
          <li class="td-sidebar-nav__section-title">
            <a href="/docs/contribution-guidelines/" class="align-left pl-0 pr-2 td-sidebar-link td-sidebar-link__section">Contribution Guidelines</a>
          </li>
          <ul>
            <li class="collapse show" id="docscontribution-guidelines">
            </li>
          </ul>
        </ul>
      </li>
    </ul>
  </ul>
</nav>

Where is this structure defined aside of https://github.com/google/docsy/blob/master/layouts/partials/sidebar-tree.html?
If we want to change this, we also have to change the CSS!
Are there other functions of Docsy, which could get effected?

@narrenfrei
Copy link
Contributor Author

I think, I've found where the structure is defined - or I have to say, I think, I've understand the concept of the template called "section-tree-nav-section" in Docsy's partial "sidebar-tree.html".

I have to say, I'm quiet new to HUGO, Docsy and also the language Go. So I took a look into other HUGO themes and found a similar concept at Zdoc using sections in the header menu and the folder structure in the sidebar.

For me it look like, they've chosen a better way to generate the sidebar tree menu: https://github.com/zzossig/hugo-theme-zdoc/blob/master/layouts/partials/main/sections/list-menu.html
In this partial they also defined a template (called "render-menu") and they use it in the range of pages. I have to say, they also don't use the correct syntax of nested ULs, but at least the don't generate unnecessary ULs and ILs.

@LisaFC
Copy link
Collaborator

LisaFC commented Feb 16, 2021

Yes, that's the right section in sidebar-tree.html The only issues I can see are that there are uls that are direct children of uls (which in pure syntax terms is "incorrect" but works in every browser I've ever tried it in) and I think we have an extra layer of uls because of the recursive way the tree is generated.

That said, even if there are syntax errors, it works, we have had literally no issues about it generating a working TOC. I'm very reluctant to make changes that might break people's sites for no reason except "generating syntactically correct HTML". I'll do a quick check and see if I can remove a ul layer and still have a working TOC with the same CSS.

@narrenfrei
Copy link
Contributor Author

I'm also taking a deeper look on the syntax and how it is generated. You've already called the 2 point: (1) the not "really" nested ULs and (2) the extra layer. I will report the solution I will use for our doku here, if it's OK for you. And then we can discuss, if it's also useful for docsy.

And I really understand, that you don't want to change something, that would break all Docsy installations out there ;-)

@LisaFC
Copy link
Collaborator

LisaFC commented Feb 16, 2021

Yes, from a quick poke around we'd need to rewrite the CSS both for styling/indentation and the collapsible sections as well as rewrite that bit of templating.

@narrenfrei
Copy link
Contributor Author

Do you know, if changes in the markup also would have some side effects (maybe JS)?

@snunez1
Copy link

snunez1 commented Feb 17, 2021

This might be a good opportunity to address #342 and hopefully #348 whilst making changes to the TOC.

@narrenfrei
Copy link
Contributor Author

narrenfrei commented Feb 17, 2021

I've committed a suggestion for partial sidebar-tree.html:

  • reordered nested ul
  • removed the extra ullayer
  • added class to count menu depth
  • added class to mark if li has a sibling ul or not
  • No CSS changes should be necessary (hopefully)

https://github.com/narrenfrei/docsy/blob/restructure-sidebar-tree-menu/layouts/partials/sidebar-tree.html

At the mentioned issues #342 and #348 are not addressed. If I find time, I'll take a look in the next days.
At the moment my suggestion for #449 is NOT built in in the branch above. So we can discuss this two issues separated.

@narrenfrei
Copy link
Contributor Author

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

@LisaFC
Copy link
Collaborator

LisaFC commented Feb 19, 2021

Ooh this is really great! Do you have a PR (which will give us a Netlify preview in our user guide) or other preview to look at?

@narrenfrei
Copy link
Contributor Author

In the last days I worked on 3 issues: #348 #449 and this one
My suggestions for all 3 issues are already build in with this commit
narrenfrei@0b93b8f
in this branch:
https://github.com/narrenfrei/docsy/blob/restructure-sidebar-tree-menu/layouts/partials/sidebar-tree.html
You can access a running example at: https://narrenfrei.github.io/docsy-dev-page

Is a comined PR for all 3 issues OK?

I'm also working on #342. Do you want to wait for my suggestion for this issue too?
I'd prefer to devevelop #342 in an seperate branch in my repo and make also a seperate PR.

@narrenfrei
Copy link
Contributor Author

I've added also the update for external links in section-index.html #449
narrenfrei@81e27d8

@narrenfrei
Copy link
Contributor Author

narrenfrei commented Mar 10, 2021

I've made a PR #475
For the other mentioned issues in this discussion I will make a separate PR when #475 is OK for everyone.

The suggestion in PR #475 for this issue is another one than mentioned above (https://github.com/narrenfrei/docsy/blob/restructure-sidebar-tree-menu/layouts/partials/sidebar-tree.html). Because I've found some more things, that could be improved in my opinion, I've rebuild the "section-tree-nav-section" within sidebar-tree.html nearly from scratch (and also the sidebar.html a little bit).

I will close now the issue, because further discussion should taken place in #475.

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

No branches or pull requests

3 participants