Question

I'm trying to refactor some php templates using the alternative syntax so they're more readable for designers (like me): http://getkirby.com/blog/php-templates

But I'm stuck refactoring a view template for concrete5's navigation block - the autonav. This is what I have which works...

<ul class="main-nav-left">
<? foreach($navItems as $ni) : ?>
  <li class="dropdown <?= ($ni->classes) ? $ni->classes : '' ?>">

  <? if ($ni->isEnabled) : ?>
    <a class="dropdown-toggle <?= $ni->classes ?>" href="<?= $ni->url ?>"><?= $ni->name ?></a>
  <? else : ?>
    <span class="<?= $ni->classes ?>"><?= $ni->name ?></span>
  <? endif ?>

  <? if ($ni->hasSubmenu) : ?>
    <ul class="dropdown-menu">
  <? else : ?>
    </li><?= str_repeat('</ul></li>', $ni->subDepth); ?>
  <? endif ?>

  </li>
<? endforeach ?>
</ul>

But I don't understand why </ul></li> doesn't get output over and over again. Wouldn't str_repeat just repeat those closing tags until $ni->subDepth ends?

Questions:

How do the opening <li> & <a> tags for the sub menu items get output?

Is there a way to refactor that str_repeat line into a foreach loop so the submenu portion is more readable?

Any pointers in the right direction would be much appreciated - I know very little PHP BTW.

Cheers

Ben

Was it helpful?

Solution

I actually wrote that version of the template. I too care greatly about making markup readable for designers (and developers), and believe it or not what you see there is probably about the simplest it can possibly be. (If you want your eyes to bleed, check out how it used to look: https://github.com/concrete5/concrete5/blob/bbdd9cb9acd31b6595c665cc1316eb76712f0e0b/web/concrete/blocks/autonav/view.php ).

The high-level reason why it's not simpler than it is: because the default template for the autonav block must accommodate a LOT of different situations for a LOT of different sites. Because it is the default navigation menu structure for every Concrete5 site, it has to be general enough to handle any possible site structure (which, in Concrete5 means any number of levels and sub-levels, alias pages, pages that redirect to their children, pages that are "hidden from nav", pages that open up in new windows, etc.).

Specifically, the str_repeat line is in there to facilitate any number of sub-levels that may exist in the sitemap structure. Doesn't matter how many levels deep the sitemap goes... 2 levels, 3 levels, 10 levels, whatever... that one line of code closes all the sub-menus regardless of how deep they are.

The method shown in the example Kirby docs that you linked to are definitely easier to read and understand:

<?php if($page->hasChildren()): ?>
    <ul>
        <?php foreach($page->children() as $child): ?>
            <li><a href="<?php echo $child->url() ?>"><?php echo $child->title() ?></a></li>
        <?php endforeach ?>
    </ul>
<?php endif ?>

...but they are limited to only working if the site is 1- or 2-levels deep. If you know for a fact that your users will only ever add 1 sub-level of pages to the sitemap, then you can totally get away with that (and you totally should -- one of the great things about C5 is that it is designed to have things like the nav menus overridden and customized for each site and doing so is much easier than in other CMS's). But as soon as someone adds a sub-sub-page, now your menu will not work. You could fix this by adding another level to that code:

<?php if($page->hasChildren()): ?>
    <ul>
        <?php foreach($page->children() as $child): ?>
            <li>
                <a href="<?php echo $child->url() ?>"><?php echo $child->title() ?></a>
                <?php if ($child->hasChildren()): ?>
                    <ul>
                        <?php foreach ($child->children() as $grandchild): ?>
                            <li>
                                <a href="<?php echo $grandchild->url() ?>"><?php echo $grandchild->title() ?></a>
                            </li>
                        <?php endforeach; ?>
                    </ul>
                <?php endif; ?>
            </li>
        <?php endforeach ?>
    </ul>
<?php endif ?>

But then what happens if there's another sub-level? You wind up with this:

<?php if($page->hasChildren()): ?>
    <ul>
        <?php foreach($page->children() as $child): ?>
            <li>
                <a href="<?php echo $child->url() ?>"><?php echo $child->title() ?></a>
                <?php if ($child->hasChildren()): ?>
                    <ul>
                        <?php foreach ($child->children() as $grandchild): ?>
                            <li>
                                <a href="<?php echo $grandchild->url() ?>"><?php echo $grandchild->title() ?></a>
                                <?php if ($grandchild->hasChildren()): ?>
                                    <ul>
                                        <?php foreach ($grandchild->children() as $greatgrandchild): ?>
                                            <li>
                                                <a href="<?php echo $greatgrandchild->url() ?>"><?php echo $greatgrandchild->title() ?></a>
                                            </li>
                                        <?php endforeach; ?>
                                    </ul>
                                <?php endif; ?>
                            </li>
                        <?php endforeach; ?>
                    </ul>
                <?php endif; ?>
            </li>
        <?php endforeach ?>
    </ul>
<?php endif ?>

You can see that there is a lot of repetition going on here, and this is why the Concrete5 autonav template is structured the way it is. That one str_repeat call replaces a theoretically infinite number of nested menus!

Apologies for the long-winded introduction (I really love these kinds of questions, thanks for asking!)... here are answers to your specific questions:

I don't understand why </ul></li> doesn't get output over and over again. Wouldn't str_repeat just repeat those closing tags until $ni->subDepth ends?

Yes, str_repeat does output the closing tags until $ni->subDepth ends, and this is its exact purpose. The underlying issue here is that while the sitemap is a nested hierarchy, the list of "nav items" that is being looped through in the code is a flat list. So somehow this "flat list" of pages needs to have information that denotes what level (or depth) each item in the list lives at in the sitemap structure. The way it does this is with the $ni->subDepth variable (which is not a built-in PHP thing nor is it a built-in Concrete5 thing... rather it is custom for this autonav template and is set in the block's controller, which lives in another file). The "subDepth" denotes the number of levels difference between "this" page and the "next" page in the nav menu. For most menu items, the difference is "0" (if the next item in the list is at the same level as the current item). But for the last item of a sub-menu, the difference is "1" (because the next nav menu item is 1 level higher than the current one... because the current one is the last item in a dropdown menu so the next item starts a new top-level menu). Occasionally the difference could be 2 or 3 (or more), which happens if you have a sub-menu whose last item is another sub-menu... then the difference between the last item of the sub-sub-menu and the next item could be "2" (because it's the last item in a sub-sub-list, so the next item starts a new top-level menu). SO... the str_repeat is a way to output the proper number of closing tags, regardless of how many levels away the current nav item is from the next one in the list.

How do the opening <li> & <a> tags for the sub menu items get output?

They get outputted the same way that the opening tags for the top-level menu items get outputted! This is the "magic" of this template... it is a way to output a hierarchical nav menu using a "flat" list and without repeating your markup over and over again for each possible sub-level. So that one line you have for the opening <li> tag:

<li class="dropdown <?= ($ni->classes) ? $ni->classes : '' ?>">

...and the one line you have for the <a> tag:

<a class="dropdown-toggle <?= $ni->classes ?>" href="<?= $ni->url ?>"><?= $ni->name ?></a>

...are what's used for EVERY item in the menu, regardless of what "depth" the item is at. Neat! (But admittedly confusing at first :)

Is there a way to refactor that str_repeat line into a foreach loop so the submenu portion is more readable?

Yes, it is possible IF you know that there will only be 1 or 2 levels of the sitemap hierarchy. But as more and more levels start appearing, the code balloons out of control, and then the str_repeat thing becomes more readable (in my opinion at least... but "readable" is in the eye of the reader so definitely do what works for you).

So there you have it... an in-depth analysis of the Concrete5 autonav template :) I hope I haven't bored you to tears, and that it's at least somewhat understandable. For the record, I think "small CMS's" like Kirby, Perch, Couch, etc. are really great systems, but there is a tradeoff between simplicity and flexibility, so different tools will be more appropriate for different jobs.

Best of luck!

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top