Composite data structure

[alexsegura]

Hello,

As agreed with @djfm, I sort of backported this branch to be retrocompatible: https://github.com/PrestaShop/blocktopmenu/commits/feat/starter-theme

This introduces a composite data structure for the menu items: now theme developers can completely redefine the HTML structure of the menu using Smarty.

{function name="blocktopmenu_menu" items=array()}
    {foreach $items as $item}
    <li {if $item.selected}class="sfHover"{/if}>
        <a href="{$item.data.link}" title="{$item.data.name}">{$item.data.name}</a>
        {if $item|count > 0}
        <ul>
            {blocktopmenu_menu items=$item.children}
            {if $item.type == 'category-thumbnails'}
            <li class="category-thumbnail">
                {foreach $item.images as $image}
                <div>
                    <img class="imgm" src="{$image.src}" alt="{$image.alt}" title="{$image.title}" />
                </div>
                {/foreach}
            </li>
            {/if}
        </ul>
        {/if}
    </li>
    {/foreach}
{/function}

<ul>
    {blocktopmenu_menu items=$items}
</ul>

To keep things retrocompatible, the $MENU variable is still assigned, using SuperfishNodeVisitor to replicate the legacy behavior.

I still need to make a few adjustments, but please start reviewing.

[kpodemski]

Nice job!

Why /src/ instead of /classes/? :)

It will be nice if you'll add additional classes like product_ID, category_ID etc.

[alexsegura]

@kpodemski /src is more standard I think, most modern libraries use this convention.

Regarding additionnal attributes, I would prefer using data attributes, like:

<li data-type="category" data-id="1">...</li>

But this is not the goal of this PR. In any case it allows the theme developer to entirely modify the HTML, so at least it is possible.

[kpodemski]

How you'll check that data-type is category? We don't have such a information in $item

[alexsegura]

Yes, you can do $item.type

[kpodemski]

Oh, yes, ok, you're right 👍

[djfm]

Order of arguments in assertions should be "expected" then "actual", not "actual" then "expected". It's easy to remember, it's reverse alphabetical order ;)

[alexsegura]

Ho that's true

[djfm]

This looks nice, but now feat/starter-theme is out of sync :) We should forward-port your back-port.
Could you make the PR on the dev branch instead please? The release process for modules is dev then master. We'll ask our QA team to test this version then.

[alexsegura]

Oops every time I forget about the dev branch! Here it is #43

@djfm I know, sorry but your code was too futuristic for the next PrestaShop 1.6.x release 😄
However, it should be easy to just implement the WidgetInterface on top of my commits.

[djfm]

Yes and remove the search feature again, then re-do the caching part (If I'm not mistaken, blocktopmenu caches the rendered HTML, which includes the class that marks the active page. I think it is better to cache the structure of the menu and decorate it with the "active" flag on the appropriate nodes at page load, which is what I did on feat/starter-theme). But I like the visitor and iterator stuff so it is worth it.