Composite data structure

[alexsegura]

Hello,

Reopening PR #42 on the dev branch

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.

[alexsegura]

I just changed the visibility of createNode to protected, and added the id of the item when available.

This allows overrides of this module to add additional data to the node.
Example to add images to category nodes:

protected function createNode($type, array $data = array())
{
    if (BlocktopmenuNode::TYPE_CATEGORY === $type) {
        $category = new Category($data['id'], $this->context->language->id);
        $data['image'] = $this->context->link->getCatImageLink($category->link_rewrite, $category->id, 'small_default');
    }

    return parent::createNode($type, $data);
}

[drzraf]

please make createCategoryNode public (see #40). It's very useful to be able to reuse such a helper.

[alexsegura]

Ok for protected

[drzraf]

why not public? It's a very useful HTML menu generator?

[alexsegura]

Because it is only here for backward compatibility: the module still assigns the $MENU variable with HTML code. Future versions should not generate the HTML anymore and this method should be deprecated ASAP.
If you want to change the HTML code, you should do it in the view, not in the controller.

[drzraf]

very nice!
(update: just had a look at #42 were it had already been discussed)

[alexsegura]

Will this be merged one day ? @Quetzacoalt91 @gaillafr @tchauviere

[alexsegura]

You will never merge this PR, isn't it ?

[drzraf]

... and that would be a very bad signal to the community. We all have in mind one (or a couple) of PHP software having gone that awful road.

I'm also waiting PSP input for too long and too much bugfixes or PR.

[prestarocket]

+1

[alexsegura]

[alexsegura]

[xBorderie]

Got feedback from the team for you:

What @alexsegura has done is a really good thing. Now we just need to make sure it works fine, then we can merge it.
By "works fine", I mean that it must not break the users' menus when they update that module to this new version.
Afterwards, ideally, we should port this to version 1.7, so that we don't have to maintain 3 different versions of this module.

There you go. The Core devs is aware of this PR, and it is approuved. It "only" needs a code-review and a QA review which, now that we've reached the alpha 3 stage for 1.7, we should soon finally find time to do properly.

Thank you for your patience!

[alexsegura]

You are welcome, better late than never 👍

I tried my best to keep it BC, the $MENU variable is still available, but I may have forgotten some edge cases. Waiting for the review, I just hope it won't take 6 months to complete 😄