? Success

User tests: Successful: Unsuccessful:

avatar Mathewlenning
Mathewlenning
25 Sep 2015

What is this?

This is a very simple class that can generate valid HTML elements.

What problem is it intended to solve?

If you look in all of our JHTML classes you'll see many very ugly and unreadable PHP/HTML mashups. This simple class allows us to git rid of those long lines of manually input html and replace them with a manipulable PHP object.

Easy Example

JHtml::iframe line 272

Before

    public static function iframe($url, $name, $attribs = null, $noFrames = '')
    {
        if (is_array($attribs))
        {
            $attribs = JArrayHelper::toString($attribs);
        }

        return '<iframe src="' . $url . '" ' . $attribs . ' name="' . $name . '">' . $noFrames . '</iframe>';
    }

After

    public static function iframe($url, $name, $attribs = null, $noFrames = '')
    {
        if (!is_array($attribs))
        {
            $attribs = array();
        }

        $attribs['src'] = $url;
        $attribs['name'] = $name;

        $iframe = new JHtmlElement('iframe', $attribs,$noFrames);

        return $iframe;
    }

By removing the raw html string variables from the code we make it more readable and easier to understand.

Complex Example

JHtmlGrid::action line 39

Before

    public static function action($i, $task, $prefix = '', $text = '', $active_title = '', $inactive_title = '', $tip = false, $active_class = '',
        $inactive_class = '', $enabled = true, $translate = true, $checkbox = 'cb')
    {
        if (is_array($prefix))
        {
            $options = $prefix;
            $active_title = array_key_exists('active_title', $options) ? $options['active_title'] : $active_title;
            $inactive_title = array_key_exists('inactive_title', $options) ? $options['inactive_title'] : $inactive_title;
            $tip = array_key_exists('tip', $options) ? $options['tip'] : $tip;
            $active_class = array_key_exists('active_class', $options) ? $options['active_class'] : $active_class;
            $inactive_class = array_key_exists('inactive_class', $options) ? $options['inactive_class'] : $inactive_class;
            $enabled = array_key_exists('enabled', $options) ? $options['enabled'] : $enabled;
            $translate = array_key_exists('translate', $options) ? $options['translate'] : $translate;
            $checkbox = array_key_exists('checkbox', $options) ? $options['checkbox'] : $checkbox;
            $prefix = array_key_exists('prefix', $options) ? $options['prefix'] : '';
        }

        if ($tip)
        {
            JHtml::_('bootstrap.tooltip');

            $title = $enabled ? $active_title : $inactive_title;
            $title = $translate ? JText::_($title) : $title;
            $title = JHtml::tooltipText($title, '', 0);
        }

        if ($enabled)
        {
            $html[] = '<a class="btn btn-micro' . ($active_class == 'publish' ? ' active' : '') . ($tip ? ' hasTooltip' : '') . '"';
            $html[] = ' href="javascript:void(0);" onclick="return listItemTask(\'' . $checkbox . $i . '\',\'' . $prefix . $task . '\')"';
            $html[] = $tip ? ' title="' . $title . '"' : '';
            $html[] = '>';
            $html[] = '<i class="icon-' . $active_class . '">';
            $html[] = '</i>';
            $html[] = '</a>';
        }
        else
        {
            $html[] = '<a class="btn btn-micro disabled jgrid' . ($tip ? ' hasTooltip' : '') . '"';
            $html[] = $tip ? ' title="' . $title . '"' : '';
            $html[] = '>';

            if ($active_class == "protected")
            {
                $html[] = '<i class="icon-lock"></i>';
            }
            else
            {
                $html[] = '<i class="icon-' . $inactive_class . '"></i>';
            }

            $html[] = '</a>';
        }

        return implode($html);
    }

After

    public static function action($i, $task, $prefix = '', $text = '', $active_title = '', $inactive_title = '', $tip = false, $active_class = '',
        $inactive_class = '', $enabled = true, $translate = true, $checkbox = 'cb')
    {
        if (is_array($prefix))
        {
            $options = $prefix;
            $active_title = array_key_exists('active_title', $options) ? $options['active_title'] : $active_title;
            $inactive_title = array_key_exists('inactive_title', $options) ? $options['inactive_title'] : $inactive_title;
            $tip = array_key_exists('tip', $options) ? $options['tip'] : $tip;
            $active_class = array_key_exists('active_class', $options) ? $options['active_class'] : $active_class;
            $inactive_class = array_key_exists('inactive_class', $options) ? $options['inactive_class'] : $inactive_class;
            $enabled = array_key_exists('enabled', $options) ? $options['enabled'] : $enabled;
            $translate = array_key_exists('translate', $options) ? $options['translate'] : $translate;
            $checkbox = array_key_exists('checkbox', $options) ? $options['checkbox'] : $checkbox;
            $prefix = array_key_exists('prefix', $options) ? $options['prefix'] : '';
        }

        $title = $enabled ? $active_title : $inactive_title;
        $title = $translate ? JText::_($title) : $title;

        $onClick = 'return listItemTask(\'' . $checkbox . $i . '\',\'' . $prefix . $task . '\')';
        $attribs = array('class' => 'btn btn-micro disabled jgrid',
                          'href' => 'javascript:void(0);',
                          'onclick' => $onClick,
                          'title' => $title);

        $element = new JHtmlElement('a', $attribs);

        if($inactive_class == 'protected')
        {
            $inactive_class = 'lock';
        }

        $icon = $element->addChild('i', array('class' => 'icon-' . $inactive_class));

        if ($tip)
        {
            JHtml::_('bootstrap.tooltip');
            $element->addClass('hasTooltip');
            $element->addAttribute('title',JHtml::tooltipText($title, '', 0));
        }

        if ($enabled)
        {
            $element->removeClass('disabled')->removeClass('jgrid');

            if($active_class == 'publish')
            {
                $element->addClass('active');
            }

            $icon->removeClass('icon-' . $inactive_class)->addClass('icon-' . $active_class);
        }

        return $element;
    }

The main benefit here is that we don't have to manually create the html element, which will reduce human error.

The secondary benefit is that classes and attributes can be manipulated, which will reduce the number of if/else statements we need through out the JHtml, JFields, JLayout classes.

The third benefit is to readability of the code.

avatar Mathewlenning Mathewlenning - open - 25 Sep 2015
avatar Mathewlenning Mathewlenning - change - 25 Sep 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Sep 2015
Labels Added: ?
avatar Mathewlenning
Mathewlenning - comment - 25 Sep 2015

Travis has spoken. I expected as much. Unfortunately it is passed my bedtime and I won't be back until Monday UTC +9. So the style checks will have to wait until then.

avatar Bakual
Bakual - comment - 25 Sep 2015

I agree the current way of filling a $html array and then imploding looks ugly, however to make the HTML more readable I would prefer a JLayout approach where we directly write the HTML. With <?php ... ?> tags mashed in. It's also easier to change/override.

avatar rdeutz
rdeutz - comment - 25 Sep 2015

I agree with @Bakual JLayout is the way we should go.

avatar dgt41
dgt41 - comment - 25 Sep 2015

I will agree with @mbabker's previous comment on a similar pr: "do not mess jhtml with jlayout". Jhtml is already overiddable so throwing another overriddable layer will only make things more complex.

avatar zero-24 zero-24 - change - 25 Sep 2015
Category Libraries
avatar Bakual
Bakual - comment - 25 Sep 2015

I see pro and cons there using JLayouts (or at least a simple layout file). In regards to readability, nothing tops a file where the HTML is written clear like in the (J)Layout files.
I'm not in favor of creating yet another JHtml class which only returns HTML output. I'd prefer the existing (ugly, but intuitive) way over that new class.

avatar Mathewlenning
Mathewlenning - comment - 25 Sep 2015

First just to clarify, when I say "Ugly", I don't mean aesthetically displeasing. I mean cognitively unpleasant. Or in other words wasting cognitive energy without any real benefit.

Although you might not have notices, every time you deal with one of those mashups, your mind is forced to run through the code and generate the resulting HTML before you can even start working with it.

If you change the code, then your mind has to run the script again. Which in my option is very wasteful.

While I understand that it is always more intuitive to just keep doing what we've been doing, template files are procedural code and procedural code always leads to bloat.

This 362 lines of OOP has the potential to reduce the JHtml files by more than 1000 lines easily. This is an insanely conservative estimate IMO. That is even before we mention what it can do for JLayout.

Also since we can return an object instead of a string, it would give developers better control of the output without requiring a full override just to add a conditional class or attribute.

The interface is also very similar to the DOM element interface, so anyone that understands HTML/JavaScript would have no problem using the class.

Full Documentation

You want to add a class

    $htmlElement->addClass('className');

You want to remove a class

    $htmlElement->removeClass('className');

You want to add an attribute

    $htmlElement->addAttribute('onclick', $value);

You want to remove an attribute

    $htmlElement->removeAttribute('onclick');

You want to set the InnerHtml

    $htmlElement->setInnerHtml($innerHtml);

You want to get the InnerHtml as a string

    $htmlElement->getInnerHtml();

You want to get the InnerHtml as an array

    $htmlElement->getInnerHtml(false);

You want to add something to the InnerHtml (Before)

    $htmlElement->addInnerHtml($innerHtml, true);

You want to add something to the InnerHtml (After)

    $htmlElement->addInnerHtml($innerHtml);

You want to add a child element to the object (Before)

    $htmlElement->addChild($tagName, $attributes, $innerHtml, true);

You want to add a child element to the object (After)

    $htmlElement->addChild($tagName, $attributes, $innerHtml);

You want to render the HTML

    echo $htmlElement;

That is it, I've now documented the entire interface. Is there any part of this that you believe is beyond the capabilities of someone with even beginner level HTML skills?

If used with JLayouts

JLayouts is looking a lot better than the last time I looked at the code. So great work to everyone! But there are still some implementations that this class could improve.

Specifically the whole libraries.cms.html.bootstrap implementation. That is 6 nearly empty files just to render a simple tab set.

avatar mbabker
mbabker - comment - 26 Sep 2015

If you're going to build HTML inside objects, IMO this is probably the way to do it. For those who venture outside Joomla world, AuraPHP has a HTML package with an entire API doing what this one class does in a more verbose way (there are classes for many HTML tags).

Many of the JHtml helpers already returns essentially a rendered HTML string. For B/C, you couldn't change them to return this JHtmlElement object directly so from that aspect there isn't much code gain other than a "cleaner" internal API that's building the rendered HTML. And you would still need to make use of JHtml's override system to change it unless you did all the rendering via JLayouts, which makes this object pointless and inherently complicates our API by adding more potential override patterns.

I'd actually be more inclined to take this and refactor JHtml with Joomla 4 so that it returns this object instead of a rendered string than to make use of JLayout in the JHtml API. It actually solves a lot of the issues we have in what might be the most consistent and friendly way to everyone. You don't have to override a JHtml helper (which gets complex because now you have to write plugins and maintain your own full PHP implementation of that function) or do complex string replacements on what's already rendered to make a small change. The output that the object does create can be overridden with relative ease by folks who may not be fluent with complex PHP operations (like was pointed out, the interface is consistent with JavaScript DOM manipulation). Overall win in my book.

avatar Fedik
Fedik - comment - 26 Sep 2015

Many of the JHtml helpers already returns essentially a rendered HTML string. For B/C, you couldn't change them to return this JHtmlElement object directly ...

not realy, if JHtmlElement has __toString() method then I see no problem with b/c on this point :wink:

I cannot say how JHtmlElement useful will be, but by looking around: we have JUri that almost perfect for build/edit the links ... but people still use strings and strpos($link, '?') for edit them :smile:

@Mathewlenning I think you missed 2 important methods in JHtmlElement: hasClass() and hasAttribute() and maybe getAtribute ... also class is Attribute, so I think it is a big mistake split it into separated property

avatar Fedik
Fedik - comment - 26 Sep 2015

so, here is my summary :neckbeard:

  • Class is Attribute, I think should stay in $attributes['class'] = array('clas1', 'class2')
  • Need two more methods hasClass() and hasAttribute() .. without this JHtmlElement not very usefull
  • Please keep terminologi clean: innerHtml is a string and only string, child is an object and only object. So I would suggest throw away $innerHtml (and related methods) and use $text + get/setText() methods. Constructor become __construct($tagName, $attributes = array(), $text = null)
  • get/setChildren() should accept/return only instance(s) of another JHtmlElement

here some quick test that this class should pass for me :smile:

$p = new JHtmlElement('p', array('class' => 'text', 'data-foo' => null, 'data-bar' => false), 'My text');
echo $p; // produce: <p class="text" data-foo="null" data-bar="false">My text</p>
var_dump($p->hasAttribute('data-foo')); // return TRUE

$div = new JHtmlElement('div', array('id' => 'text-wrapper'), 'Another text');
$div->addChildren($p);
echo $div; // produce: <div id="text-wrapper">Another text<p class="text" data-foo="null" data-bar="false">My text</p></div>

$div->addText('More text!!!');
echo $div; // produce: <div id="text-wrapper">Another text<p class="text" data-foo="null" data-bar="false">My text</p>More text!!!</div>

$div->setText('Clear them all!!!');
echo $div; // produce: <div id="text-wrapper"><p class="text" data-foo="null" data-bar="false">My text</p>Clear them all!!!</div>

$div->setRemoveChildren($p);
echo $div; // produce: <div id="text-wrapper">Clear them all!!!</div>

$div->setText($p);
echo $div; // produce: <div id="text-wrapper">&lt;p class=&quote;text&quote; data-foo=&quote;null&quote; data-bar=&quote;false&quote;&gt;My text&lt;/p&gt;Clear them all!!!</div>

$div->setText('<div></div>');
echo $div; // produce:<div id="text-wrapper">&lt;div&gt;&lt;/div&gt;</div>

$div->setText('');
$div2 = new JHtmlElement('div', array(), $div);
echo $div2; // produce: <div>&lt;div id=&quote;text-wrapper&quote;&gt;Clear them all!!!&lt;/div&gt;</div>

$div->addChildren('some text'); // Produce Error, because accepted only another JHtmlElement or array of them

if($p->hasClass('test'))
{
    $p->addClass('another-test');
}
$p->addClass('another-test'); // try duplicate
echo $p; // produce: <p class="text another-test" data-foo="null" data-bar="false">My text</p>

I know it is hard :smile:

For a bit simplify the thing, setText() can accept 2 param setText($value, $escape = true), but result should be always String:

$p = new JHtmlElement('p', array(), 'My text');
$div = new JHtmlElement('div');

$div->setText($p);
echo $div->getText(); // produce: &lt;p&gt;My text&lt;/p&gt;

$div->setText($p, false);
echo $div->getText(); // produce: <p>My text</p>
avatar mbabker
mbabker - comment - 26 Sep 2015

not realy, if JHtmlElement has __toString() method then I see no problem with b/c on this point

The JHtml::link() method return a string HTML tag for a link element. Changing this method to return a JHtmlElement object is a B/C break because it changes the signature of the API. Internal to the method, it could build the HTML element using the JHtmlElement class but its return must be return (string) $element;. New JHtml helpers could be implemented returning the JHtmlElement object instead of fully assembled HTML, but existing helpers could not change the return until 4.0.

avatar Mathewlenning
Mathewlenning - comment - 27 Sep 2015

@Fedik

First thanks for taking the time to share your thoughts. I like the idea about hasClass and hasAttribute and will be implementing them when I start cleaning up the styles for Travis.

I disagree with some of your other suggestions, so allow me to justify my position on each of your points.

Class is Attribute, I think should stay in $attributes['class'] = array('clas1', 'class2')

Although class is an attribute, it is a special attribute. No other attribute allows for multiple values or is space delimited.

Using your suggestion here would mean that we'd have to complicate addClass, removeClass, and hasClass. I'm a simplest thing that works kind of guy and this works just fine. If however you have some reason other than preference that would justify your suggestion here, I would love to hear it.

I would suggest throw away $innerHtml (and related methods) and use $text + get/setText() methods. Constructor become __construct($tagName, $attributes = array(), $text = null)

I intentionally used getInnerHtml() and setInnerHtml because they are part of the Dom interface. get/setText() is not part of the Dom interface, so we'd be forcing everyone to learn a new interface with no benefit IMO.

get/setChildren() should accept/return only instance(s) of another JHtmlElement

There actually isn't a child property in this class. Everything is part of the innerHtml array. My goal here wasn't to rebuild the DOM in PHP, if you need that kind of power then there are already a lot more complex/powerful solutions available. This is just a very simple class that does what it was intended to do very well.

avatar Mathewlenning
Mathewlenning - comment - 27 Sep 2015

@mbabker I agree completely that JHtml couldn't return these objects until J4. A string is a string.

avatar wilsonge
wilsonge - comment - 27 Sep 2015

I like this. It's very similar to the JGrid concept (https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/grid/grid.php) - which when used the right way is stupidly powerful

10408cb 28 Sep 2015 avatar Mathewlenning Doh!
avatar Mathewlenning
Mathewlenning - comment - 28 Sep 2015

Alright! I love the color green.

@wilsonge Thanks. I haven't used JGrid, but I'll defiantly check it out in the near future. I build my own grid generator using this elements class and a pure XML definition file and it is AWESOME.

I can literally generate a Joomla list view with one xml file now (Custom filters and all), but I don't think it is something that is needed in the core (at least not yet).

The reason I made this pull request is that cleaning up those ugly PHP/HTML mix ups will make it easier to see more opportunities for improvement. Maybe just a very small step, but like you said "when used the right way, stupidly powerful."

avatar Mathewlenning
Mathewlenning - comment - 29 Sep 2015

I had a mini epiphany last night. One of the limitations of this class was that there was no way to actually get the child elements out of the parent element once they were in there. Unless you made a reference using JHtmlElement::addChild and caught the return.


    $element = new JHtmlElement('div', array('class' => 'container'));

    // Now we can work with the child reference
    $reference = $element->addChild('a',  $attributes, $innerHtml);

However with the input from @Fedik. I added hasAttribute and hasClass methods.

Then I realized we could search the innerHtml array and return all child classes that had an attribute.

So I added a new method called ** getChildrenByAttribute($name) **

This was awsome, but I really wanted to search for the ID values too.
So I added an optional value to the method ** getChildrenByAttribute($name, $value = null) **

The result is that we can now search the parent element and get an array of all child elements by an attribute name,or by attribute value.


    $element = new JHtmlElement('div', array('class' => 'container'));
    $element->addChild('a', array('class' => 'btn btn-large', 'href' => 'someThing'));

    // result is an array with a reference all child elements with the btn class
    $result = $element->getChildrenByAttribute('class', 'btn');

    // result is an array with a references to all child element with the href attribute
    $result = $element->getChildrenByAttribute('href');

    // result is an array with a reference to all child elements 
    //with a href attribute value of someThing
    $result = $element->getChildrenByAttribute('href', 'someThing');

    // result in empty array
    $result = $element->getChildrenByAttribute('onclick');

I love OOP =^D

avatar jrseliga
jrseliga - comment - 30 Sep 2015

Strangely enough over the last few months at agilesdesign we've been putting together a generic PHP library providing an interface to object-orient the building/rendering of Documents and their Elements.

https://github.com/agilesdesign/Document

The ultimate goal is for frameworks like Bootstrap 3 simply being classes that extend directly from our Elements. It's still in the early stages, the Document\Html class is still very much a work in progress but please check it out. You will notice there is already the complete Bootstrap 3 library available in the repository. We've been putting in extra time the last few weeks in preparation for suggesting our library to the Joomla! 4 Working Group.

avatar phproberto
phproberto - comment - 1 Oct 2015

The more we go away from html markup the less frontenders will use Joomla. This is a backender solution to a frontender problem. Same than @jrseliga proposal.

There is a very easy solution. I'll use a menu as example and current JLayout markup but of course this can be improved using prefixes, etc.:

// Render a menu
echo JLayoutHelper::render('joomla.menu.list');

That layout is just a proxy that will load another layout. Contents of the joomla.menu.list would be something like:

// Render the menu
echo JLayoutHelper::render('bootstrap3.menu.list');

And the bootstrap3 layout would be something like:

// Load any assets required by this framework
JHtml::_('bootstrap3.framework');

?>
<ul class="nav nav-pills">
  <li role="presentation" class="active"><a href="#">Home</a></li>
  <li role="presentation"><a href="#">Profile</a></li>
  <li role="presentation"><a href="#">Messages</a></li>
</ul>

Benefits:

  • It's a solution that already exists and already can be overriden. Not reinventing the wheel.
  • You can change the active framework replacing calls in the joomla layouts. So migration from bootstrap3 to bootstrap 4 only needs to replace JLayoutHelper::render('bootstrap3.menu.list'); with JLayoutHelper::render('bootstrap4.menu.list');
  • Frontenders change html markup and don't have to deal with complex markup-agnostic solutions
avatar Mathewlenning
Mathewlenning - comment - 1 Oct 2015

Layouts is an incomplete solution. This class could be the missing piece.

Having 1000 layouts each with 10 ~ 15 lines and a bunch of PHP/HTML mash ups is not a maintainable solution.

Sincerely,
Mathew Lenning

P.S. This message was sent via iPhone, so please forgive any errors

On Oct 1, 2015, at 5:35 PM, Roberto Segura notifications@github.com wrote:

The more we go away from html markup the less frontenders will use Joomla. This is a backender solution to a frontender problem. Same than @jrseliga proposal.

There is a very easy solution. I'll use a menu as example and current JLayout markup but of course this can be improved using prefixes, etc.:

// Render a menu
echo JLayoutHelper::render('joomla.menu.list');
That layout is just a proxy that will load another layout. Contents of the joomla.menu.list would be something like:

// Render the menu
echo JLayoutHelper::render('bootstrap3.menu.list');
And the bootstrap3 layout would be something like:

// Load any assets required by this framework
JHtml::_('bootstrap3.framework');

?>

Benefits:

It's a solution that already exists and already can be overriden. Not reinventing the wheel.
You can change the active framework replacing calls in the joomla layouts. So migration from bootstrap3 to bootstrap 4 only needs to replace JLayoutHelper::render('bootstrap3.menu.list'); with JLayoutHelper::render('bootstrap4.menu.list');
Frontenders change html markup and don't have to deal with complex markup-agnostic solutions

Reply to this email directly or view it on GitHub.

avatar drmmr763
drmmr763 - comment - 1 Oct 2015

I see this as being a much better abstraction to some things we already have in Joomla. I think we have lists, tables (grid?) and some other basic HTML rendering that all in all are pretty clunky. They're missing features, you can't inject certain parameters or attributes (eg: JForm was missing placeholder for a bit).

A class like this gives all those things something to build off of as a solid base.

That said - I too dislike the idea of "rendering" html in PHP. The good thing is that a responsible developer can use a class like this to append to the normal MVC architecture:

  • Controller (user action)
  • Model (get data for user action)
    • - Prepare View - Little in between step that prepares the markup for rendering. Eg: build or populate all the objects to be rendered. Rendering should never be done here!
  • View (Render)

Another side benefit is with all the proper objects populated via a class, something like a twig interface could be created to pull all these objects and render them outside of our php layouts, or even return rendered html via ajax. (not recommended but at least another use-case).

avatar mbabker
mbabker - comment - 1 Oct 2015

We really need to be careful with how much we use JLayout. Truth be told, if we start having layouts that are single line proxies to different framework specific layouts, I think we're screwing something up royally. Right now, using JLayout is a major win in places like JToolbar and JForm. I'm not 100% sold that using JLayout in JHtml is efficient considering JHtml already has its own override system; it feels like bad code structure to me if we move to a system where part of a method can be overridden in one way (the layout overrides via JLayout) or the method can be overridden in full (via JHtml::register()) and not even use the layout.

This in itself isn't a one stop solution for all the CMS' rendering. At best, it can deal with simple to moderate HTML strings. So maybe it works for things like JHtml::image() or the tooltip helpers. I don't see it as a full replacement for the helpers rendering a Bootstrap slider element.

There is no winning with a pure code solution for our rendering platform. At the same time, we've already got examples in the CMS about what happens if we misuse JLayout (see some of the JHtmlBootstrap pieces or the layouts that have a JViewLegacy object injected into them), proving we're willing to abuse our code in ways that makes no sense. Whatever path forward we take, we need to be careful about it and not introduce terrible architectural decisions into it just because we can.

avatar phproberto
phproberto - comment - 1 Oct 2015

@mbabker of couse JLayout is not the solution for everything. We currently are using it in the wrong way in a lot of places.

About JHtml: in my opinion it shouldn't use JLayout but JHtml shouldn't be used to render HTML (ironic because its name) only to load assets.

Things like JHtml bootstrap tabs should be directly deprecated and replaced by the real markup where they are currently used. You know what? Bootstrap already has a nice documentation that everybody knows! So save time and resources and make frontenders happy allowing them to use markup directly.

About this PR the sample usage looks nicer for a backender but definitely worse for a frontender (no html markup at all).

IMO that layouts/joomla/content/categories_default.php shouldn't exist at all!

avatar mbabker
mbabker - comment - 1 Oct 2015

Before JLayout, JHtml was the way to go to produce shared HTML snippets. Because let's face it, copy/pasting all that MooTools style tab/slider markup in every layout would've sucked. Now we have JLayout, so shuffling things around definitely makes sense. For the life of me though, I never ever ever ever ever ever ever want to see us accept another layout like addtabscript or endtabset.

avatar dgt41
dgt41 - comment - 1 Oct 2015

@mbabker @phproberto Out of curiosity, these form fields Jlayout PRs #7702, #5871, #5655 and #5654 are ok or am I abusing the layouts there?

avatar phproberto
phproberto - comment - 1 Oct 2015

About the field sample usage I'll show you what I use. JFormField would have a method like:

    /**
     * Get the data that is going to be passed to the layout
     *
     * @return  array
     */
    protected function getLayoutData()
    {
        // Label preprocess
        $label = $this->element['label'] ? (string) $this->element['label'] : (string) $this->element['name'];
        $label = $this->translateLabel ? JText::_($label) : $label;

        // Description preprocess
        $description = !empty($this->description) ? $this->description : null;
        $description = !empty($description) && $this->translateDescription ? JText::_($description) : $description;
        $hiddenLabel = empty($options['hiddenLabel']) && $this->getAttribute('hiddenLabel');
        $hint = $this->translateHint ? JText::_($this->hint) : $this->hint;
        $debug    = !empty($this->element['debug']) ? ((string) $this->element['debug'] === 'true') : false;

        return array(
            'autofocus'   => $this->autofocus,
            'class'       => trim($this->class . ' form-field'),
            'debug'       => $debug,
            'description' => $description,
            'disabled'    => $this->disabled,
            'element'     => $this->element,
            'field'       => $this,
            'hidden'      => $this->hidden,
            'hiddenLabel' => $hiddenLabel,
            'hint'        => $hint,
            'id'          => $this->id,
            'label'       => $label,
            'multiple'    => $this->multiple,
            'name'        => $this->name,
            'onchange'    => $this->onchange,
            'onclick'     => $this->onclick,
            'readonly'    => $this->readonly,
            'required'    => $this->required,
            'size'        => $this->size,
            'value'       => $this->value
        );
    }

Then child fields only need to set specific data they need. My list field is something like:

    /**
     * Get the data that is going to be passed to the layout
     *
     * @return  array
     */
    protected function getLayoutData()
    {
        $layoutData = parent::getLayoutData();
        $layoutData['options'] = (array) $this->getOptions();

        return $layoutData;
    }

So it inherits all the data is prepared by the parent field and adds its own.

The getInput() method can't be simpler:

    /**
     * Method to get the field input markup for a generic list.
     * Use the multiple attribute to enable multiselect.
     *
     * @return  string  The field input markup.
     */
    protected function getInput()
    {
        $layout = !empty($this->element['layout']) ? (string) $this->element['layout'] : $this->layout;

        // Ensure field meets the requirements
        if (empty($layout) || !$this->validateRequirements())
        {
            return;
        }

        return $this->getRenderer($layout)->render($this->getLayoutData());
    }

And that's all. The layout receives the data and anybody can decide how to process it. So you have a clean roles separation. Backender prepares the data (classes) and frontenders style it (layouts).

For views I use the same logic. A parent base view with a getLayoutData() that prepares basic things and then views for each kind of common views that joomla uses.

ItemForm view is like:

    /**
     * Get the data that is going to be passed to the layout
     *
     * @return  array
     */
    public function getLayoutData()
    {
        $this->model = $this->getModel();
        $this->item  = $this->model->getItem();
        $this->state = $this->model->getState();
        $this->form  = $this->model->getForm();

        $extraData = array(
            'model' => $this->model,
            'item'  => $this->item,
            'state' => $this->state,
            'form'  => $this->form
        );

        return array_merge(parent::getLayoutData(), $extraData);
    }

In fact I don't have to change display method at all. It's in the base view. Compare that with our current display() methods. And it's fully B/C because only new views will extend the new base view. Their display method won't call getLayoutData() so you can be happy living in the past.

This was about rendering HTML but I wanted to show why I'm 100% convinced JLayout is the way to go.

avatar phproberto
phproberto - comment - 1 Oct 2015

@dgt41 almost perfect for me. Replace that hardcoded $displayData with $displayData = $this->getLayoutData(); (see my previous TL DR; ) to allow child fields to use inheritance and it would be perfect :+1:

avatar mbabker
mbabker - comment - 1 Oct 2015

You're both on point with one of my biggest groans about the Form package in general, getting the inline markup out of those classes. I won't preach on the API structure here because my initial thoughts take a "Framework first" approach and that's not exactly the most popular topic in these parts :wink:

avatar dgt41
dgt41 - comment - 1 Oct 2015

"Framework first" for the Forms will be part of our Athens sprint, @nikosdion will redo the logic and I’ll do the easy part the layouts. At least that’s the plan ????

avatar nikosdion
nikosdion - comment - 1 Oct 2015

Daniele will also help with the logic and he'll be providing most of the POC we'll discuss in Athens

avatar Mathewlenning
Mathewlenning - comment - 1 Oct 2015

O.k. I think a lot of people are making some assumptions on how this class would be used because it is in the JHtml folder. So first I want to state that the reason it is in the JHtml folder is because of its name. Other than that it has no connections with JHtml or its legacy.

So if everyone can put the pitchforks down for a moment and actually consider what I'm saying, we could resolve the whole template override problem in a way that satisfies the front-end devs need for easy overriding without HTML/PHP gunk.

Lets imagine for a moment that we deleted everything in the JHtml folder with the exception JHtmlElement. To me this wouldn't be a bad move for Joomla. (of course we can't, but imagination has no boundaries)

So now we have just JLayouts. O.k. one system for overrides is better than two. So we've made progress. But AS-IS JLayouts is nothing more than old logic from JView by a different name. But what if it was more than that?

The BIG idea

What if the layouts didn't use the .php extension, but used the .xml extension.

For example what if joomla-cms/layouts/joomla/toolbar/standard.php was written something like this

<button onclick="@doTask" class="btn @classes">
    <span class="@icon-class"></span>
    <text translate="true">
</button>

Please don't get hung up on the syntax as this is just an example. All the actual details would have to be hashed out.

Anyway so then a back-end developer uses a JLayout call like

 echo JLayoutHelper::render('joomla.toolbar.standard', $displayData);

Rather than just buffering and returning the echo result of standard.php, it reads the standard.xml file and generates a JHtmlElement instance that matches the standard.xml structure and replaces the variable with values from the corresponding $displayData.

Front-end Dev Overrides

If you've read this far than thank you. So lets assume a front-end dev wants to override the output of our example layout. Maybe he wants to add a data- attribute to it

He copies standard.xml and moves it to the same directory that he would move the standard.php file now.

Then he edits the xml file like so

<button onclick="@doTask" class="btn @classes" data-customized="value">
    <span class="@icon-class"></span>
    <text translate="true">
</button>

** That's it! ** now every time the standard layout is called it will have the data-customized attribute.

Back-end devs

What about developers? Maybe our extension developer wants to make sure that a class is included in the standard buttons because she has some JS script that depends on it. So instead of echoing out the layout directly, she captures the return value in a variable. Since it is her extension, no one should have any objections right.

$standardElement =  JLayoutHelper::render('joomla.toolbar.standard', $displayData);

//add the required class
$standardElement->addClass('js-identifier');
echo $standardElement;

Now she can rest easy knowing that regardless of which template Joomla is running or what the end-user might do to the layout.xml file the required class name will be there and can drop the n number of lines of JavaScript that he/she was using to do sanity checks after the page was rendered.

That is what I SEE

So when I say PHP in PHP, HTML in HTML that vision is what I'm talking about.
If everyone still thinks that the current approach is a better solution then what I just described, then there isn't anything else to say.

@nikosdion said that I I should put it up for a vote, so lets do that. If you don't think the solution I just described is worth pursuing just cast your vote with a -1 and I'll close this PR and go do something productive.

If you think that what I just described could be the solution +1 me and lets start working on how to make it a reality.

avatar Mathewlenning
Mathewlenning - comment - 4 Oct 2015

I took a few hours yesterday and build a POC of the system I described. https://github.com/Mathewlenning/simple-widget

avatar Mathewlenning
Mathewlenning - comment - 16 Oct 2015

So I don't want to keep kicking a dead horse here, but you want to see how I'd do a bootstrap modal?

<?xml version="1.0" encoding="UTF-8"?>
<template>
    <div id="@modal_id" class="modal hide fade">
        <div class="modal-header">
            <button type="button" class="close" data-dismiss="modal" aria-hidden="true"><![CDATA[&times;]]></button>
            <h3><var data-source="@modal_header"/></h3>
        </div>
        <div class="modal-body">
            <layout data-source="@modal_body"/>
        </div>
        <div class="modal-footer">
            <layout data-source="@modal_footer"/>
        </div>
    </div>
</template>

Granted it isn't 100% HTML, so there is a learning curve, but I think a front-end could figure it out rather quick. And with proper documentation, anyone that could figure out how to do overrides today won't have a problem.

Did you notice I'm using JLayouts to render the modal body and modal footer content?

It's too bad this won't make it into Joomla!, because like so many of my other proposals this too would be a game changer.

Did I mention that It is already working? =^P

Here I'm rendering the joomla.system.message layout in my modal.
xml_modal

avatar mbabker
mbabker - comment - 16 Oct 2015

Great. Now deal with a complex layout with calls to various PHP methods. https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/searchtools/default/bar.php is a good one to look at to see the types of very basic things you have to be able to do in a rendering engine in Joomla.

avatar Mathewlenning
Mathewlenning - comment - 16 Oct 2015

O.k.

<layout data-source="joomla.searchtools.default.bar"/>

Does that work?

avatar Mathewlenning
Mathewlenning - comment - 16 Oct 2015

But I'd prefer to pull that layout apart first it looks like there are a few different widgets in there

avatar Mathewlenning
Mathewlenning - comment - 16 Oct 2015

For the time being I'm focusing my attention on replacing JForm. My ideas aren't much use to the project, but at least I can make my life easier. Happy Joomla!ng

avatar Mathewlenning
Mathewlenning - comment - 17 Oct 2015

Well replacing JForm was easier that I thought. Of course I've only built the editor widget, but most of the other ones can be done in plain HTML without too much trouble. So it works for my use case.

<?xml version="1.0" encoding="UTF-8"?>
<template>
    <form action="/bu-products/administrator/index.php?option=com_babelu_testing&amp;view=default&amp;layout=form" method="post" name="adminForm" id="adminForm" class="form-validate">
        <div class="form-horizontal">
            <div class="row-fluid">
                <div class="span12">
                    <fieldset class="adminform">
                        <legend>Standard Settings</legend>
                        <div class="control-group">
                            <div class="control-label">
                                <label id="jform_title-lbl" for="jform_title" class="hasTooltip required" title="TITLE_TOOL_TIP">
                                    <jtext>TITLE_LABEL</jtext><span class="star"><![CDATA[&nbsp;*]]></span>
                                </label>
                            </div>
                            <div class="controls">
                                <input type="text" name="jform[title]" id="jform_title" value="@title" class="required" size="30" required="required"/>
                            </div>
                        </div>
                        <div class="control-group">
                            <div class="control-label">
                                <label id="jform_alias-lbl" for="jform_alias" class="hasTooltip" title="ALIAS_TOOL_TIP">
                                    <jtext>ALIAS_LABEL</jtext>
                                </label>
                            </div>
                            <div class="controls">
                                <input type="text" name="jform[alias]" id="jform_alias" value="@alias" class="inputbox" size="45" placeholder="Auto-generate from title"/>
                            </div>
                        </div>

                        <div class="control-group">
                            <div class="control-label">
                                <label id="jform_state-lbl" for="jform_state" class="hasTooltip" title="STATE_TITLE">
                                    <jtext>STATUS_LABEL</jtext>
                                </label>
                            </div>
                            <div class="controls">
                                <select id="jform_state" name="jform[state]" class="inputbox chzn-done" size="1" value="@state">
                                <option value="1">Published</option>
                                <option value="0">Unpublished</option>
                                <option value="2">Archived</option>
                                <option value="-2">Trashed</option>
                            </select>
                            </div>
                        </div>
                    </fieldset>
                </div>
            </div>
            <div class="row-fluid">
                <div class="span12">
                    <fieldset class="adminform">
                        <legend>
                            <jtext>DESCRIPTION_LABEL</jtext>
                        </legend>
                        <div>
                            <editor id="jform_description" name="jform[state]" class="inputbox" value="@description" width="100%" height="550px"/>
                        </div>
                    </fieldset>
                </div>
            </div>
            <input type="hidden" name="jform[default_id]" value="@id"/>
            <input type="hidden" name="task" value=""/>
            <session_token/>
        </div>
    </form>
</template>
$displayData = array();
$displayData['title'] = 'My Title';
$displayData['alias'] ='My Alias';
$displayData['state'] = '0';
$displayData['id'] = '1';
$displayData['description'] = "<h1>My Header</h1><p>I often ask myself what am I going to do?</p>";

$acl = new aclTest();
$rendered = $widgetRenderer->render('form', $acl, $displayData);


foreach($rendered AS $html)
{
    echo $html;
}

And the output =^D

form_template

avatar Fedik
Fedik - comment - 17 Oct 2015

about JForm I hope it was a joke :smile:

avatar Mathewlenning
Mathewlenning - comment - 17 Oct 2015

I wouldn't call it a JForm anymore. Now it's just a form with dynamic values. ????????????

Sincerely,
Mathew Lenning

P.S. This message was sent via iPhone, so please forgive any errors

On Oct 17, 2015, at 4:50 PM, Fedir Zinchuk notifications@github.com wrote:

about JForm I hope it was a joke


Reply to this email directly or view it on GitHub.

avatar dgt41
dgt41 - comment - 17 Oct 2015

@Mathewlenning Can we find a way to implement this for JHtml. I tried the JLayout way and for the most parts the results are not good (moderate comment)

avatar nikosdion
nikosdion - comment - 17 Oct 2015

Then, Dimitris, I wish you good luck with Joomla! 4. Reinventing HTML in a
way that designers can't use is a brilliant way to make this thing
unusable.

avatar dgt41
dgt41 - comment - 17 Oct 2015

@nikosdion check out #8105. Do you like the overrides?
These overrides make sense: #8104, #8101, #8088 as we can move (or even replace) parts of JHtml with JLayouts. But the use of JLayouts for the creation of simple html output seems to fall sort. Of course it might be that I used (or abused) it the wrong way!

avatar nikosdion
nikosdion - comment - 17 Oct 2015

Layouts are HTML which can be overridden by designers. What I see here is a
reinvention of HTML in XML. So, you want to implement XHTML from scratch? I
mean, this XML here is not even framework agnostic. You have to define the
DOM and classes. JForm on the other hand is designed to be framework
agnostic, with the framework dependent part abstracted in pure HTML and PHP
layouts using JLayout.

That was the decision in Denmark and reconfirmed in Athens. If we are to go
about and make random direction changes I am not interested. Have fun
reinventing HTML.

avatar dgt41
dgt41 - comment - 17 Oct 2015

I guess I can rewrite some JHtml, let’s say JHtml::select, to make more sense when used with JLayouts and NOT try to get one more way of doing things...

avatar nikosdion
nikosdion - comment - 17 Oct 2015

Select is the simplest you can get. I would argue it needs no JLayout
conversion. Some of the grid stuff does need. All our tabs, sliders and
Bootstrap specific JHtml helpers definitely do JLayout abstraction.

Abstraction is implemented to serve a purpose. It is NOT its own purpose.
We write code for users to use, not to mentally jerk off as I put it in
Athens.

avatar Mathewlenning
Mathewlenning - comment - 17 Oct 2015

@nick do whatever you want mate. But I would argue that JForm, JHtml and for a lesser part JLayouts are all reinventing HTML. My approach is actually going back to HTML.

Is the form TEMPLATE framework dependent? Yes.

But so are the BS2 layout templates.

However the underlying widget system I built doesn't even know about the "DOM" or the template.

Crazy part is I can use JLayouts, JHtml, JForm inside my my widget system without changing Joomla or corrupting my system.

And I'm pretty sure I could use twig, blade or any other template engine too.

I know this all sounds impossible right? HtmlElement was the difficult part. I had to refactor that class about 20 times before I was satisfied. But after that it really wasn't that difficult.

Especially after having my work insulted by yet another Joomla Guru. So thank you for helping me reach the next level. Believe it or not you've helped me make this happen more than you know.

Sincerely,
Mathew Lenning

P.S. This message was sent via iPhone, so please forgive any errors

On Oct 17, 2015, at 5:53 PM, Nicholas K. Dionysopoulos notifications@github.com wrote:

Layouts are HTML which can be overridden by designers. What I see here is a
reinvention of HTML in XML. So, you want to implement XHTML from scratch? I
mean, this XML here is not even framework agnostic. You have to define the
DOM and classes. JForm on the other hand is designed to be framework
agnostic, with the framework dependent part abstracted in pure HTML and PHP
layouts using JLayout.

That was the decision in Denmark and reconfirmed in Athens. If we are to go
about and make random direction changes I am not interested. Have fun
reinventing HTML.

Reply to this email directly or view it on GitHub.

avatar phproberto
phproberto - comment - 17 Oct 2015

I will say it again. Using JLayout in JHtml is wrong. Replaceall the logic that uses JHtml to render stuff with layouts is the right way. So if you want to use layouts in JHtml to keep B/C calling the same layouts that are going to replace JHtml calls is ok but it will look wrong (as it does in your PRs).

avatar dgt41
dgt41 - comment - 17 Oct 2015

@phproberto the idea was to totally replace them with jlayout calls, but that can not happen before 4.0. See #8088
@deprecated 4.0 instead of JHtml::_('batch.item'); use JLayoutHelper::render('joomla.html.batch.item', array('extension' => 'com_XXX'));

avatar brianteeman
brianteeman - comment - 2 Aug 2016

I am closing this - it is never going to get updated or implemented


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7948.

avatar brianteeman brianteeman - change - 2 Aug 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-08-02 19:44:10
Closed_By brianteeman

Add a Comment

Login with GitHub to post a comment