picocms / Pico

Pico is a stupidly simple, blazing fast, flat file CMS.
http://picocms.org/
MIT License
3.81k stars 616 forks source link

Nested listing of posts #656

Closed notakoder closed 1 year ago

notakoder commented 1 year ago

I am trying to achieve a nested list of posts since some articles have a section or part that it belongs to like in a book.

Here is an example

I have been using the following snippet to list articles normally.

<ul>
{% for page in pages %}
{% if (page.id starts with "doc/") and not page.hidden %}
    <li><a href="{{ page.url }}">{{ page.title }}</a></li>
{% endif %}
{% endfor %}
</ul>

To achieve what I want, I tried this,

<ul>
{% for page in pages|sort_by("page.meta.ListPosition") %}
{% if (page.id starts with "doc/") and not page.hidden %}
    {% if (page.meta.Part) %}
        <ul>
        <li>{{ page.meta.Part }}
            <ul>
                <li><a {% if page.id == current_page.id %} class="active"{% endif %} href="{{ page.url }}">{{ page.title }}</a></li>
            </ul>
        </li>
        </ul>
    {% else %}
        <li><a {% if page.id == current_page.id %} class="active"{% endif %} href="{{ page.url }}">{{ page.title }}</a></li>
    {% endif %}
{% endif %}
{% endfor %}
</ul>

The order of listing is correct, but because the if condition is checked at every for loop iteration, section/part is repeatedly listed for all posts. Like so:

The approximate English code of what I want to achieve is: for each page in pages, list the pages as unordered/ordered lists, but if the pages have a part, list those pages in a nested list. I have no idea how to achieve the but branch of the code in Pico.

mayamcdougall commented 1 year ago

I haven't tested this code, so you might need to debug it a little.

So here's the breakdown.

First, we set an empty variable called current_part. This is to keep track of what part we're on. We're setting it outside of the loop that way it persists from loop to loop.

Next there's your original For and If lines.

Then we switch things up. We start with the closing code, that way if needed, we can close out one list before immediately opening another during the same loop. I'm not 100% certain on your pages layout, and whether there would always be a non-parted entry in-between the parted ones, so this makes sure we always close up the previous list before starting another.

We check if current_part is set (meaning we're inside a list, though we'll get to that below), but doesn't match the current page's Part (either because it doesn't have one, or it has a different one), in which case, we should close out the list.

We also clear out the current_part variable here, so we can start fresh in the next condition. (You'll notice these conditions are all separate and not elseif conditions. This way they can occur in sequence and not just run one or the other.)

So next is the opening block. If current_part value isn't set, but the current page has a Part set then we need to print our list opening block. We also store the Part string as current_part that way next loop, we'll know if we're still in the same Part section or not.

Next, we print our page list item. Yep, no conditions attached, we just print it. No matter if we're in an inner list we created or outside in the main one, the code was the same. So this is acting a little like an else... but it's more of an "always", lol. πŸ˜‚

Then, finally I realized while writing this comment that we probably needed a way to close out the list in-case we ended on a page with a Part. So the last code block I added just checks if we're on the last iteration of the loop (loop.last), and if current_part is still set, we close out the list. I don't like that this bit is duplicated, but there didn't seem like a simple way to avoid that.

<ul>
{% set current_part = "" %}
{% for page in pages|sort_by("page.meta.ListPosition") %}
    {% if (page.id starts with "doc/") and not page.hidden %}

        {% if current_part and current_part != page.meta.Part %}
            {% set current_part = "" %}
                    </ul>
                </li>
            </ul>
        {% endif %}

        {% if not current_part and page.meta.Part %}
            {% set current_part = page.meta.Part %}
            <ul>
                <li>{{ page.meta.Part }}
                    <ul>
        {% endif %}

        <li><a {% if page.id == current_page.id %} class="active"{% endif %} href="{{ page.url }}">{{ page.title }}</a></li>

        {% if loop.last and current_part %}
            {% set current_part = "" %}
                    </ul>
                </li>
            </ul>
        {% endif %}

    {% endif %}
{% endfor %}
</ul>

It's been awhile since I've written any Twig code, so hopefully it works okay. Let me know if you have any issues with it. πŸ˜‰

notakoder commented 1 year ago

Ok this works! You've explained the code very well too, although it will take time for me to get them.

However, I noticed one thing. For each part, a new ul is created instead of just another li tag. So, we have one extra nested listβ€”3 instead of 2. Here is the HTML semantic for the code.

<ul>
    <li><a class="active" href="#">Introduction/a></li>
    <li><a href="#">How to use</a></li>
    <ul>
        <li>Part 1
            <ul>
                <li><a href="#">Part 1 Page 1</a></li>
                <li><a href="#">Part 1 Page 2</a></li>
                <li><a href="#">Part 1 Page 3</a></li>
            </ul>
        </li>
    </ul>
    <ul>
        <li>Part 2
            <ul>
                <li><a href="#">Part 2 Page 1</a></li>
                <li><a href="#">Part 2 Page 2</a></li>
                <li><a href="#">Part 2 Page 3</a></li>
            </ul>
        </li>
    </ul>
    <li><a href="#">Conclusion</a></li>
    <li><a href="#">Appendix</a></li>
</ul>

Frankly speaking, we don't need Part 1 and Part 2 to be inside an ul. It can just be another li element of the outer ul. Just noticed, hence mentioned.

Thank you again for your help @mayamcdougall. I am closing the issue. But if you have a fix for the above issue, do post it as per your convenience.

notakoder commented 1 year ago

Frankly speaking, we don't need Part 1 and Part 2 to be inside an ul. It can just be another li element of the outer ul. Just noticed, hence mentioned.

Just noticed a good reason why parts must be just an other li instead of a whole new ul. If you change the listing from unordered list to ordered list for the want of numbered bullets, it messes up the numbering.

mayamcdougall commented 1 year ago

Frankly speaking, we don't need Part 1 and Part 2 to be inside an ul.

Oh, whoops. I did say I didn't test it. πŸ˜…

That just came from your example and I didn't think too hard about it.

Are you sure you don't want those in a ul? Properly, together I mean. Not in a new one for each item. πŸ€¦πŸ»β€β™€οΈ

It seems like you'd want to have this layout rather than your original example:

There's an outer ul, then one nested in the How to use page, then another ul nested under each part. Seems like the Semantic way to do it. There are actually three levels of ul, we just don't want to be closing it between the parts.

But I can adjust the logic to whichever works for you, two or three levels of ul.

As far as support for ol goes, then I think you'd definitely want three levels.

mayamcdougall commented 1 year ago

Alright, so... the logic started hurting my head, so I made some convenience variables this time which made the code a lot cleaner. I do all the checks at the beginning to determine what state we're in. Taking the logic that used to be in the separate if blocks, we have three conditions:

There was another variable called same_part, probably the most obvious "state" to be in... but it wasn't actually used for anything in the logic, so I removed it.

I've also renamed current_part to prev_part and moved setting it to the end of the loop because it made more sense this way (and calling it current_part was starting to get confusing).

<ul>
{% set prev_part = "" %}
{% for page in pages|sort_by("page.meta.ListPosition") %}
    {% if (page.id starts with "doc/") and not page.hidden %}

        {% set first_part = not prev_part and page.meta.Part %}
        {% set new_part = prev_part and page.meta.Part and prev_part != page.meta.Part %}
        {% set end_part = prev_part and not page.meta.Part %}

        {% if new_part or end_part %}
                    </ul>
                </li>
        {% endif %}

        {% if end_part %}
            </ul>
        {% endif %}

        {% if first_part %}
            <ul>
        {% endif %}

        {% if first_part or new_part %}
                <li>{{ page.meta.Part }}
                    <ul>
        {% endif %}

        <li><a {% if page.id == current_page.id %} class="active"{% endif %} href="{{ page.url }}">{{ page.title }}</a></li>

        {% if loop.last and page.meta.Part %}
                    </ul>
                </li>
            </ul>
        {% endif %}

        {% set prev_part = page.meta.Part %}

    {% endif %}
{% endfor %}
</ul>

I've fixed numerous oversights just while writing this comment, so there could definitely be more issues to debug. I just don't feel like setting up a test environment for it, so let me know if something explodes. πŸ˜…

(Edit: Also, this is assuming you do actually want three levels of ul like I mentioned in my previous comment.)

notakoder commented 1 year ago

(Edit: Also, this is assuming you do actually want three levels of ul like I mentioned in my previous comment.)

No, we need only two ul. Three is what we have and two is what we should ideally have. I think I mentioned it on my first reply (So, we have one extra nested listβ€”3 instead of 2).

It seems like you'd want to have this layout rather than your original example:

I think that the confusion is because of the "How to use heading", which I choose arbitrary. Part 1 and Part 2 does not belong to "How to Use"; they are their own separate sections. What I want is indeed the below structure that now uses generic headings

Technically, all pages are in the same directory. And it's only in the listing that those belonging to a section are grouped together in order to merely convey a hierarchy to the reader. So, we ought to have only two listings. The outer one and the inner one; instead of three.

Anyway, since it's only a minor issue, I wouldn't suggest that you refactor anything at this point. I am also considering whether Sections 1 & Section 2 should be clickable links with their own pages (all we need to do is add the tag). Currently, section pages (Section 1 and Section 2) doesn't have any its own pages and I was using Page 3 and Page 6 as introduction to their respective sessions.

There are also other considerations I am making such as whether it can mimic a headless architecture, etc. I'll update.

Thanks anyway. You usual put in a lot of work on my issues.

mayamcdougall commented 1 year ago

No, we need only two ul.

Off the top of my head, it should be as simple as removing:

{% if end_part %}
    </ul>
{% endif %}

{% if first_part %}
    <ul>
{% endif %}

As well as removing the extra </ul> from the loop.last block.

I'm not going to mentally step through all the logic again, but I'm like 99% sure that's all it would take (provided the original example works correctly).

This should put the part headers in the root level of the list. But yeah, they looked like they were supposed to be sections of "How to use". Especially given your original code example. πŸ˜…

I am also considering whether Sections 1 & Section 2 should be clickable links with their own pages

Well... I mean... they will be links in the code I wrote, since everything uses the same inner <li>, no matter which level of the list they are on. Wouldn't be hard to just add some extra logic there though.

Thanks anyway. You usual put in a lot of work on my issues.

No problem, lol. I enjoy trying to solve problems in Twig. 😁

(It's powerful enough to do fun stuff like this, but not so powerful as to get bogged down with complex code).

notakoder commented 1 year ago

Off the top of my head, it should be as simple as removing...

That's right. It does work!

they will be links in the code I wrote, since everything uses the same inner

  • , no matter which level of the list they are on. Wouldn't be hard to just add some extra logic there though.

  • We can add the <a> tag inside the <li> and make each part name a link, indeed. However, this causes a double listing of the part page at the same ul level: one with no nesting and the other a nesting. This is because of the line <li><a {% if page.id == current_page.id %} class="active"{% endif %} href="{{ page.url }}">{{ page.title }}</a></li> . We can certainly filter such pages to be avoided. But since I am reconsidering the structure of the listing, let's defer this issue to another time.

    Thank @mayamcdougall