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

RFC: Menu: only expand active hierarchy #195

Closed
wants to merge 1 commit into from

Conversation

robhoes
Copy link

@robhoes robhoes commented Nov 16, 2018

Before this change, the entire menu tree below the selected top-level heading
was expanded.

This change adds some triangles on menu items at level 2 and below to indicate
which ones can be or are expanded. I didn't put them on the top level, because
I felt that it looked too cluttered and didn't add much value.

Fixes #88

Here is an example of what this looks like: https://robhoes.github.io/hugo-theme-learn/en/shortcodes/children.

Would it be desirable to add a configuration option for this, perhaps just for the new marks in the menu?

Before this change, the entire menu tree below the selected top-level heading
was expanded.

This change adds some triangles on menu items at level 2 and below to indicate
which ones can be or are expanded. I didn't put them on the top level, because
I felt that it looked too cluttered and didn't add much value.

Fixes matcornic#88
Copy link
Contributor

@coliff coliff left a comment

Choose a reason for hiding this comment

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

This works great. It's very helpful for larger docs sites. I'd like to see this merged. 👍

An option to enable/disable this feature would be desirable. For smaller sites I'd probably prefer this option to be turned off/disabled.

@matalo33 matalo33 self-requested a review December 21, 2018 00:43
@matalo33 matalo33 added enhancement Improvements to existing features need user feedback labels Dec 21, 2018
@matalo33
Copy link
Contributor

Agree, this is an excellent addition. However I believe it should be enabled only with a toggle and set to off by default to avoid breaking people's existing websites who don't expect this behaviour.

@robhoes would you be able to wrap this in a config toggle?

@robhoes
Copy link
Author

robhoes commented Jan 3, 2019

Thanks guys. Yes, I'll have a look at adding a toggle. I need to see how to do it for the CSS bit though, which would probably need a slightly bigger change.

@matalo33
Copy link
Contributor

matalo33 commented Jan 3, 2019

I had another read of this yesterday and spotted you already have a toggle named alwaysopen. Was testing toggling this on and off and it seemed to work for me even with leaving the css changes in place, but life got in the way before I finished.

@robhoes
Copy link
Author

robhoes commented Jan 3, 2019

Oh interesting. Then we just need to make that the default somehow, and also make it hide the new markers.

@matalo33
Copy link
Contributor

matalo33 commented Jan 3, 2019

My view on this, and I'm happy to help implement if you like...

  • Default setting should be off (don't want to affect behaviour of existing websites)
  • Ready to discuss this further, but personally I don't like the chevrons :). (The site I run had this implemented for a while now in a theme override rather than in the theme master, and it was too hacky to port over. It doesn't have the chevrons and I kinda prefer it). Do you think we could add a feature flag around the chevrons? Instead of true/false the feature variable could even contain the name of the font-awesome icon to display so that people could chose their own.

@frangrolemund
Copy link

FWIW, agree 100% with some sort of way to manage menu expansion - especially for large docsets. Not a fan of the auto-chevrons either, but one option you could consider that worked for my project are a pair of site parameters that operate like the pre variable, but define the open/close prefixes for these expandable items. That way they're easy to omit and they sort of flow with the rest of the layout because they come before, not after the title.

@chapmanc
Copy link

Any movement on this ? Would love to get this in ? @robhoes @matalo33

@robhoes
Copy link
Author

robhoes commented Apr 11, 2019

Sorry, I got stuck with other work. I'll see if I can update the PR soon to take into account the feedback above.

@n-rodriguez
Copy link

Hi there! Any news?

@oldenboom
Copy link

Please see issue #355 for a pull request against master to solve this issue.

@robhoes
Copy link
Author

robhoes commented Dec 19, 2019

Sorry for my lack of engagement on this, and thanks @oldenboom for picking this up. I'll close this PR now in favour of yours.

@robhoes robhoes closed this Dec 19, 2019
@jmrichardson
Copy link

Hi, I would like to implement this but not sure how to do it. Any help would be appreciated. Thanks.

@Nightshadelink
Copy link

@jmrichardson see the changes here https://github.com/matcornic/hugo-theme-learn/pull/195/files

Red lines represent deletions. Green ones represent additions.

@oldenboom
Copy link

@jmrichardson the changes suggested by @Nightshadelink won't match with the latest branch of this theme. This one will:
https://github.com/matcornic/hugo-theme-learn/pull/355/files
I've implemented this on https://www.spelenboek.nl/

@jmrichardson
Copy link

Thanks, yes, I just cloned your repo from here https://github.com/oldenboom/hugo-theme-learn and it works. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features need user feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand Sidebar
9 participants