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

rebuilt sidebar menu structure #475

Merged
merged 3 commits into from
Mar 15, 2021
Merged

rebuilt sidebar menu structure #475

merged 3 commits into from
Mar 15, 2021

Conversation

narrenfrei
Copy link
Contributor

@narrenfrei narrenfrei commented Mar 10, 2021

Basic information

  • Corrected markup of nested menu lists
  • best possible support of the existing CSS
  • additional classes to simplify project-specific layout changes
  • Cache mode of sidebar menu now caches the menu only once and not for each menu item with children separately

Addressed problems

  • structure of nested lists
  • The same IDs ($ sid) were used several times when menu items had several sub-items with several sub-items again. The ID of HTML elements should always be unique.
  • The class "active" was not consistent because the actually active <a> tag and also the corresponding parent <a> element received the class, but not grandparent elements. Now really only the active <a> element gets the class "active", but all <li> ancestors get the class "active-path".
  • With the compact menu (.Site.Params.ui.sidebar_menu_compact = true) all ancestors and descendants were visible until now. As a result, when the lowest level was active, the whole menu was visible - not really compact ;-) All the child pages were only hidden when a sub-page was opened. At the moment I would have implemented it in such a way that all ancestors, siblings and direct descendants are always shown. In addition, the .Site.Params.ui.ul_show parameter can be used to set a desired menu depth to always be visible. For example, the 1st menu level can always be displayed.
    • Is this behavior okay or should the old behavior be retained?
  • The class "collapsed" was originally used for the <a> elements of <li class = "td-sidebar-nav__section-title">. This was deleted without replacement because (1) it is not used in the CSS definitions (just "collapse") and (2) the control of the fade-in and fade-out via the <li> seems to work without any problems.

Changes

  • No additional (unnecessary) <ul> for menu items with children.
  • The classes "td-sidebar-nav__section" and "td-sidebar-nav__section-title" were merged onto an <li> element in order to correct the structure of the nested lists and nevertheless to support the existing CSS as best as possible.
  • The sidebar menu is now cached not only for each parent element (.CurrentSection.RelPermalink), but for the entire page tree (.FirstSection.RelPermalink). So the site generation for really big sites should be a little bit faster, hopefully. To make this possible, the JS in the sidebar.html was also adapted.
    • Is this behavior okay or should the old behavior be retained?

New Features

  • Introduced the class navRoot to make it easier to format the heading of the section, which is part of the <ul> structure, as "heading".
  • Counting the structure depth for nested lists (ul-0, ul-1, ...)
  • Introduction of <span> around menu links
  • New (optional) parameters in the config.toml
    • .Site.Params.ui.ul_show (default: 1): Possibility to specify a menu depth that is always visible, even with compact menus
    • .Site.Params.ui.sidebar_cache_limit (default: 2000): Limit from which the sidebar menu is cached (previously hardcoded)
    • .Site.Params.ui.sidebar_menu_truncate (default: 50): Limit from which a submenu is shortened (previously hardcoded)
  • Rewriting of the definition of "section-tree-nav-section" within sidebar-tree.html
    • One run of section-tree-nav-section per menu entry (previously Pages .IsPage were output directly). ==> Advantage only 1 definition for the markup per menu entry and therefore easier to understand and to maintain.
    • Since the "acitve" class is quite general, the <span> element of the active menu item is also marked with the "td-sidebar-nav-active-item" class.

Further steps
Because this PR is the basis for further steps, I would dedicate myself again on the other open issues I'm working on: #342, #348, #449, #457 and some CSS tweaks, when this PR is OK,

Corrected markup of nested menu lists
best possible support of the existing CSS
additional classes to simplify project-specific layout changes
@narrenfrei
Copy link
Contributor Author

I've now published 3 versions to test the frontend:

@LisaFC
Copy link
Collaborator

LisaFC commented Mar 12, 2021

This is fantastic work! I'm going to go through everything in detail now to see if I think we need any tweaks/anything else, but thanks so much for doing this.

@LisaFC
Copy link
Collaborator

LisaFC commented Mar 12, 2021

Re the caching, I think we do want your new behaviour. We had a couple of issues with slow generation for very big sites with deep content hierarchies (hence LOTS OF RECURSION generating the navbar)- eg #144 - so anything that improves that is good, and it doesn't seem to create any other unwanted issues that I can see.

Any other users have any thoughts?

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