Refactor toc() and li()

Discussions and requests related to new CMSimple features, plugins, templates etc. and how to develop.
Please don't ask for support at this forums!
Post Reply
cmb
Posts: 12717
Joined: Tue Jun 21, 2011 11:04 am
Location: Mü-Sa, RLP, DE
Contact:

Refactor toc() and li()

Post by cmb » Thu May 08, 2014 12:00 am

Hello Community,

does anybody understand what's going on inside of toc() and li()?

I pick up on this issue, because I recently commited a fix which introduced the funtion XH_buildHc(); this function is closely related to toc() and $hc is often used in templates as first argument to li() (or rather xli() of xtoc). This pointed my attraction to toc() and li() again, and IMHO both functions are way too long and too cryptic and should be refactored ASAP.

I suggest to apply "Replace Method with Method Object" on both functions first (toc() becoming the model and li() the view), and after having unravelled the algorithms to move related functionality (such as XH_buildHC()) to the respective class(es). Especially, I suggest to keep an eye on potential customizations and to create an API that allows these without restricting them to current functionality. To better understand what I mean, consider the original xtoc: this wasn't able to cater for $cf['menu']['sdoc']="parent", which was introduced with CMSimple 2.8, and had to be replaced by xtoc28--however, existing templates most likely weren't updated, so the new functionality could not be used.

Frankly, I don't know how much flexibility wrt. customization we can achieve, but we should strive for some sensible maximum. After all, I consider xtoc, AdvancedToc etc. merely as workarounds.

As we already have a quite comprehensive test suite regarding toc() and li() in place (see trunk/tests/unit/MenuTest.php), and the current API (toc() and li()) won't change, I dare to propose this refactoring for XH 1.6.3.

Christoph
Christoph M. Becker –Plugins for CMSimple_XH, but not for CMSimple 4+

svasti
Posts: 1649
Joined: Wed Dec 17, 2008 5:08 pm
Location: Bielefeld, Germany
Contact:

Re: Refactor toc() and li()

Post by svasti » Fri May 09, 2014 6:05 pm

$hc seems to be missing in the docu... even in the dev-doc I cannot find it.
When + by whom was it introduced? What does h + c stand for? headlines of content?
Anyhow, $hc is useful in drop down and fold out menus.

Would be nice to have toc() and li() more readable, and maybe integrate some functionality so that xtoc isn't necessary any more.

$cf['menu']['sdoc']="parent" seems to me a setting, which should belong to the template and not to the core. It was probably introduced, because it was a new feature that shouldn't break existing templates. As it was introduced long ago I guess that the time has come to skip this setting and making "parent" standard. Probably there are only very few templates that use a new XH version and still need $cf['menu']['sdoc'] switched off. This could easily be adjusted in the stylesheet.

cmb
Posts: 12717
Joined: Tue Jun 21, 2011 11:04 am
Location: Mü-Sa, RLP, DE
Contact:

Re: Refactor toc() and li()

Post by cmb » Fri May 09, 2014 7:30 pm

svasti wrote:When + by whom was it introduced? What does h + c stand for? headlines of content?
$hc was first introduced in CMSimple 2.3 by Peter Harteg. In CMSimple 2.8 its population was moved from rfc() to functions.php, with the comment "Backward compatibility for DHTML menus". The changelog mentions:
CMSimple version 2.8 beta 1
CMSimple hide functionality recoded - $hc, $hl and $hs are not used anymore.
CMSimple version 2.8 beta 2
Backward compatibility for ie. DHTML menus added to functions.php ($si, $hc and $hl is set).
So it seems $hc originally had a different purpose. Ah, I found the following explanation in the archived forum:
Harteg wrote:I have removed the use of $hc (hidden content) to simplify the code and rebuild the CMSimple hide functionality and made a complete recode of toc/submenu-function.
(Interestingly, $hc contained the indexes of the visible pages, though.)
svasti wrote:$cf['menu']['sdoc']="parent" seems to me a setting, which should belong to the template and not to the core. It was probably introduced, because it was a new feature that shouldn't break existing templates. As it was introduced long ago I guess that the time has come to skip this setting and making "parent" standard. Probably there are only very few templates that use a new XH version and still need $cf['menu']['sdoc'] switched off. This could easily be adjusted in the stylesheet.
Hm, I don't think that either setting of menu_sdoc will break a template (except for some very contrived cases); when set to "parent" it just adds the sdoc(s) CSS class to the ancestors of the currently selected page. Whether it belongs to the core or the template might be debatable, but as it's in the core, we may stick with it. We may consider to hide this option, though.

What I was trying to explain with the mention of menu_sdoc above, was that such new config options and other features will always need an update to any custom toc() implementation bundled with a template, so it's better if it wouldn't be necessary to have a full replacement for toc()/li(), but being able to replace a tiny bit of it's implementation. Consider version 3 of xli() for instance: this only "turns a clicked button to a clickable button" (i.e. wraps the currently selected page item in an <a> element). To accomplish that, Till had to deliver a complete copy of li() with only a small modification. If there had been a more flexible way to modify li(), xtoc would still have worked with CMSimple 2.8's menu_sdoc.
Christoph M. Becker –Plugins for CMSimple_XH, but not for CMSimple 4+

cmb
Posts: 12717
Joined: Tue Jun 21, 2011 11:04 am
Location: Mü-Sa, RLP, DE
Contact:

Re: Refactor toc() and li()

Post by cmb » Fri May 09, 2014 8:41 pm

Oops, nearly forgot the most important thing:
svasti wrote:$hc seems to be missing in the docu... even in the dev-doc I cannot find it.
Made good for it: r1272.
Christoph M. Becker –Plugins for CMSimple_XH, but not for CMSimple 4+

svasti
Posts: 1649
Joined: Wed Dec 17, 2008 5:08 pm
Location: Bielefeld, Germany
Contact:

Re: Refactor toc() and li()

Post by svasti » Sun May 11, 2014 8:43 am

cmb wrote:but as it's in the core, we may stick with it. We may consider to hide this option, though.
+1

cmb
Posts: 12717
Joined: Tue Jun 21, 2011 11:04 am
Location: Mü-Sa, RLP, DE
Contact:

Re: Refactor toc() and li()

Post by cmb » Sun Sep 21, 2014 7:16 pm

I have done some basic refactoring (r1355, r1375-r1376). To not further delay the release of CMSimple_XH 1.6.3, I will do the rest for XH 1.6.4.
cmb wrote:Consider version 3 of xli() for instance: this only "turns a clicked button to a clickable button" (i.e. wraps the currently selected page item in an <a> element). To accomplish that, Till had to deliver a complete copy of li() with only a small modification. If there had been a more flexible way to modify li(), xtoc would still have worked with CMSimple 2.8's menu_sdoc.
Now it is possible to replace this variant of xtoc with:

Code: Select all

<?php

require_once $pth['folder']['classes'] . 'Menu.php';

class MyLi extends XH_Li
{
    function renderMenuItem($i)
    {
        global $h;

        return $this->renderAnchorStartTag($i) . $h[$this->ta[$i]] . '</a>';
    }
}

function xtoc($start = null, $end = null)
{
    return toc($start, $end, array(new MyLi, 'render'));
}

function xli($ta, $st)
{
    $li = new MyLi();
    echo $li->render($ta, $st);
}

?>
svasti wrote:
cmb wrote:but as it's in the core, we may stick with it. We may consider to hide this option, though.
+1
I have not done this, because I'm not sure, if that was part of the voting. Anyway, we have Hide all folder_* config options on the roadmap for XH 1.6.4, where svasti already suggested to hide further config options, so we can do it then.
Christoph M. Becker –Plugins for CMSimple_XH, but not for CMSimple 4+

cmb
Posts: 12717
Joined: Tue Jun 21, 2011 11:04 am
Location: Mü-Sa, RLP, DE
Contact:

Re: Refactor toc() and li()

Post by cmb » Tue Jan 20, 2015 8:27 am

I have refactored a bit more (r1467+r1471). This is still far from what I consider understandable code, but due to lack of time and features (PHP 5), I'll leave it for now.
Christoph M. Becker –Plugins for CMSimple_XH, but not for CMSimple 4+

Post Reply