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

generate multi-level menus #4

Closed
oupala opened this issue Jul 24, 2015 · 7 comments · Fixed by #5
Closed

generate multi-level menus #4

oupala opened this issue Jul 24, 2015 · 7 comments · Fixed by #5

Comments

@oupala
Copy link

oupala commented Jul 24, 2015

There are many examples on the web on how to generate multi-level menus with handlebars. The problem is that they are often based on the following data structure :

var headings = [
    { id: "foo1", tag: "h1", text: "foo1 text", items: [
        { id: "foo11", tag: "h2", text: "foo11 text" },
        { id: "foo12", tag: "h2", text: "foo12 text" }
    ]},
    { id: "foo2", tag: "h1", text: "foo2 text" },
    { id: "foo3", tag: "h1", text: "foo3 text" }
];

As you can see, submenus are embedded in their parent menu.

metalsmith-headings generates a flat data structure :

var headings = [
    { id: "foo1", tag: "h1", text: "foo1 text" },
    { id: "foo11", tag: "h2", text: "foo11 text" },
    { id: "foo12", tag: "h2", text: "foo12 text" },
    { id: "foo2", tag: "h1", text: "foo2 text" },
    { id: "foo3", tag: "h1", text: "foo3 text" }
];

It is then harder to generate a correct menu with handlebars.

I'd like to get something like this in html :

<!-- toc -->
<div class="col-md-3">
    <div class="sidebar hidden-print affix-top toc" role="complementary">
        <ul class="nav sidenav">
            <li ><a href="#foo1">foo1 text</a>
                <ul class="nav">
                    <li class=""><a href="#foo11">foo11 text</a></li>
                    <li class=""><a href="#foo12">foo12 text</a></li>
                </ul>
            </li>
            <li><a href="#foo2">foo2 text</a></li>
            <li><a href="#foo3">foo3 text</a></li>
        </ul>
    </div>
</div>

As you can see, sublist ul is embedded in its parent li.

I manage to start something that works a little bit. But there is large flaws to improve. I'm not an expert in js, nor in handlebars. You can see a working test the following JSFiddle example.

Mains flaws are :

  • I'm not sure the generated html would be correct in any cases
  • html is generated by helper whereas it should be generated by the template

Feel free to share your thinking, to improve my solution, and to discuss about the data structure that metalsmith-headers should generate.

@oupala
Copy link
Author

oupala commented Aug 21, 2015

I'm now sure that the generated html is not always correct.

If you have the following headings :

var headings = [
    { id: "foo1", tag: "h1", text: "foo1 text" },
    { id: "foo2", tag: "h2", text: "foo11 text" }
];

My JSFiddle example will generate the following html :

<!-- toc -->
<div class="col-md-3">
    <div class="sidebar hidden-print affix-top toc" role="complementary">
        <ul class="nav sidenav">
            <li></li><li><a href="#foo1">foo1 text</a></li>
            <li><a href="#foo2">foo2 text</a>
    </div>
</div>

You can notice the useless <li></li> at the beginning of line 5 which should be like :

            <li><a href="#foo1">foo1 text</a></li>

You can also notice the missing </li></ul> at the end of line 6 which should be like :

            <li><a href="#foo2">foo2 text</a></li></ul>

I definetly have to find a smarter way to generate my menu. Any help gladly welcome !

Maybe @hegemonic (who is the author of the pull request that adds the tag level to metadata : #1) has an idea on this...

@oupala
Copy link
Author

oupala commented Aug 21, 2015

In addition, the second flaws mentionned in my first comment annoys me a lot :

html is generated by helper whereas it should be generated by the template

I'm pretty sure that I'm using Handlebars helpers the wrong way.

Here again, any help gladly welcome !

@jmjf
Copy link

jmjf commented Nov 18, 2015

The problem in your JSFiddle seems to be that you're outputting a </li> at the beginning of lines 31 and 37, meaning you assume an open <li> that isn't there for the first item in the list. So you need to skip the </li> for the first item.

And it seems like the headings output should be available somewhere in @options.root, so would be available in a Handlebars partial. That might let you avoid a helper altogether with a recursively called partial.

Hope that helps. If I think of anything else or get a break to look at the issue, I'll follow up. (Or maybe it spurs some ideas for others who can jump in.)

@oupala
Copy link
Author

oupala commented Nov 25, 2015

My problem is that I need to close the <li> before opening a new one.

I need to do that because I'm not closing <li> after opening it.

And that's because I need to leave the <li> open in case there is a subitem.

And unfortunately, I don't get your point about @options.root, probably because my knowledge is too weak on handlebars. Can you please give me some more information about that ? (maybe another JSFiddle... ?)

@aumouvantsillage
Copy link

metalsmith-headings generates a flat data structure :

var headings = [
    { id: "foo1", tag: "h1", text: "foo1 text" },
    { id: "foo11", tag: "h2", text: "foo11 text" },
    { id: "foo12", tag: "h2", text: "foo12 text" },
    { id: "foo2", tag: "h1", text: "foo2 text" },
    { id: "foo3", tag: "h1", text: "foo3 text" }
];

As far as I can see, the list of headings is ordered by tags, like this:

var headings = [
    { id: "foo1", tag: "h1", text: "foo1 text" },
    { id: "foo2", tag: "h1", text: "foo2 text" },
    { id: "foo3", tag: "h1", text: "foo3 text" },
    { id: "foo11", tag: "h2", text: "foo11 text" },
    { id: "foo12", tag: "h2", text: "foo12 text" }
];

To generate a multi-level menu, the list of headings should preserve the ordering.

@oupala
Copy link
Author

oupala commented Jul 11, 2017

Problem fixed using a fork: xiphiaz/metalsmith-headings (see this message).

This plugin could be fixed when pull request #5 will be merged.

@fouad fouad closed this as completed in #5 Jul 27, 2017
@oupala
Copy link
Author

oupala commented Jul 28, 2017

Very good news, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants