I owe you so so so many beers :D :D

I'll review travis errors tomorrow

I think you forgot to include the layout for the vote plugin? Vote doesn't show currently.

About the stats collecting. If I saw this correct you're doing that always? Wouldn't it make sense to only do that when debug is enabled? Otherwise we just generate overhead here for no reason.

About the prefix, I'm not sure I understand the intention here:

So for example if you are on a template that supports both bootstrap2 & bootstrap3 mode you can put in the layouts/bootstrap3 folder the specific layouts for bootstrap3.

As I understand it, the prefix (as well as suffixes, autolanguage and others) is set by the extension. How does that help a template? Or is that something a template could set globally?
Developers likely only add those if they provide different layouts themself. So most will not enable that. After all it's also a bit of a performance issue to look up ~15 override paths per layout (3 from autolanguage, 3 from autoversion, 7 from prefix plus the regular ones).
Writing this, I just wonder what exactly happens when we have both prefixes and suffixes set. Do they multiplicate? Then that would make the above ~42 (7 prefix * 6 suffix) include paths to look up... That's makes be a bit feared right now.

Generally speaking, while I like what you did here, I fear this PR got a bit big which makes it harder to test. Especially since there are so many places one would have to test things. If you could for example split it up into a part for the basic enhancement (maybe with the plugin) and another for the JForm part, that would help. If the JForm one does depend on the basic one, it may be better to do it incrementally.

Would we be able to integrate into the JForm class a method to add a prefix/suffix to the layouts for all form fields? So if I want to say load bootstrap 2/3 classes or even frameworky. I can load that for all fields in the form instead of having to override the layout path for each individual field?

Just to be an arse :D

Sorry I forgot to answer before closing the PR. Thanks for checking it.

@Bakual :

I think you forgot to include the layout for the vote plugin? Vote doesn't show currently.

Yes. I forgot the vote layout. Thanks for pointing it. I added it before closing this to be sure I have all ready here.

About the stats collecting. If I saw this correct you're doing that always? Wouldn't it make sense to only do that when debug is enabled? Otherwise we just generate overhead here for no reason.
about the

It's not a overhead to store 10 vars (counters) in an array. There is no performance issue for having it enabled it just increases counters and get microtimes. Maybe in the future if the system ges complex it will make sense. I invite you to waste your time doing performance tests.

About prefixes, suffixes: they are there for anybody that wants to use them. Of course you have to use the tools in a good way. That's the main reason to have now the layouts debug. The caching system will also help. I'll appreciate that the next time I propose something (with the tools to test it) you waste your time testing it before suggesting issues.

Generally speaking, while I like what you did here, I fear this PR got a bit big which makes it harder to test. Especially since there are so many places one would have to test things. If you could for example split it up into a part for the basic enhancement (maybe with the plugin) and another for the JForm part, that would help. If the JForm one does depend on the basic one, it may be better to do it incrementally.

This is something that needed to go together. The system was done to be a first step into the right direction and to start using the tools in the right way at the same time. 3.3 release was next week and you knew it.

I know it's not something easy to test. It wasn't to code and double check it. That's why I closed the PR. I'm not ready to rush during a week to explain why I use !empty() or discuss what should go inside a layout or not. Not prepared for a full week like that to get this merged. It's quite unmotivating to talk about the finger when it's pointing to a bigger thing.

I have lost a lot of sleeping time to get this done. I won't lose more.

@wilsonge I faced that problem but I'm not sure modifying JForm is a good move. Probably the best move here would be to search the way to allow form XML overriding. This will solve this and many other issues.

@phproberto I certainly didn't want to demotivate you and I am sorry if it came across like this.
It's hard to test something when you don't know the ins and outs of the code involved.
To test this I probably have to invest a whole evening. I'm happy to do that because I think it's a good move. But I would like to understand it prior to testing. That's why I asked the questions based on looking at your description (and briefly code) to get it right.

I've sent a new commit to add layout support for views. It's intended to replace the old system (template folders are the same). So we cannot use it on core components for now because BC but we can start using it for new views and start encouraging extension developers to use it.

To switch to the new system you have to replace in your view display method:

parent::display($tpl);

with:

echo  $this->getRenderer('default')->render(
    array(
        'items'      => $this->items,
        'pagination' => $this->pagination,
        'state'      => $this->state,
        'user'       => $this->user
    )
);

I removed the new mvc view path. For classes that use the view folder they can override the getRenderer method.

Branch rebased

OK awesome :) I've been really busy today but I'll have this tested tomorrow (afternoon/evening) :D

Review all extended JFormFieldList fields

To fully ensure BC I will review that all the fields that extend JFormFieldList work.

• Bannerclient
• CategoryEdit
• CategoryParent
• Directories
• SearchFilter
• MenuOrdering
• MenuParent
• Menutype
• Newsfeeds
• GroupParent
• Language
• Author
• Contentlanguage
• Contenttype
• Headertag
• Helpsite
• Limitbox
• Menu
• Moduletag
• Tag
• UserGroupList
• AccessLevel
• CacheHandler
• Combo
• DatabaseConnection
• FileList
• FolderList
• Integer
• Language
• Plugins
• PredefinedList
• SessionHandler
• SQL
• Category

I converted getRenderer view method to public to be able to render a view inside another view (pseudo-HMVC)

Rebased vs latest staging to resolve conflicts

Roberto - somethings weird. I can't seem to use pagination in the article manager (it works in a clean 3.2.3 install so not sure what the hell is going on) - I don't understand at all why your changes would affect that specific pagination though :/

Further to the above it also appears in banners (and is related to the search tools - as these are the two components where search tools appears to be used) - it would appear that everytime to press anything in pagination for some reason unpublish is being selected in the search tools - even when it's not open - god knows why

Thanks for testing @wilsonge

The problem with pagination was that it was identifying "" as 0. That's why unpublished was always selected

Ahh I see. I've just hit a PR into your branch that starts to deal with the unit test failures. Not sure how you want to deal with the layout changes - whether in the JLayout or in the unit tests themselves. Updates look good on a quick run through in localhost but will test more thoroughly tomorrow as pretty tired right now :)

I'm not sure neither but maybe in the unit tests is better no? To compare field properties

Hmm probably. Not quite sure how we'd go about comparing the properties though other than doing some nasty preg_replace checks or something like that :/ Will have a ponder. At least the unit test failures now are just due to the spacings in the fields

In the meantime I've found an absolutely critical bug. See my PR :P

@mbabker Any preferences on how to deal with the unit test changes which are failing cause of the extra line breaks?

OK :) Looks perfect to me!

To get this on 3.3 we have to fix the unit tests and get this tested by more people in next 1-2 days

I can do the tests. There's already 1 other test on the tracker and I can hopefully get 1 more to test. 3 testers + passing tests should be enough to set ready for review

Test done successfully.

Rebased again

Tested the suffix thing, struggled a bit about the setSuffixes('autoversion') and setSuffixes('auto language'), because one overwrites the other, I had expected that using both results in something like:

file.en.j3.php
file.lrt.j3.php

....

It's complicated but maybe worth thinking about, maybe for one of the next version something like "autoall".

Thanks Robert!

I thought about mixing both but that would be 4 x 3 = 12 file searches. I agree it can be a good thing in next versions but for now I'd ensure that the system is stable with current changes and incoming layout conversions.

PR to fix the tests is awaiting you :)

@phproberto great job as always :) I've noticed that you've added a renderData method. May be it is logical to change render to renderData in the helper?

mmm sorry @b2z I forgot to comment that feature :D

The renderData is intended to be the final rendering method (programmaticaly talking). It adds some feature commonly used on other rendering systems:

Assign variables while you create them

$renderer = new JLayoutFile('joomla.content.tags', null, array('debug' => true));

$calculatedVar = 105 / 5;

$renderer->assign('myVar', $calculatedVar);

echo $renderer->renderData(array('anotherVar', 145));

The main problem with this is that it expects that you always pass an array to the renderer and that's not BC because we now have places were we are passing view instances directly for example.

That's why I had to create a new method.

The helper is intended to be a fast call to load a layout and render it with the given data. So you can still use the old call. I think we can standarize the calls for 4.0 but not before.

I'll add this documentation to the PR text. Thanks for pointing it.

P.S.: I rebased it and fixed some conflicts with current master

@phproberto thanks, now it is clear!

So, for the tests, I think if they were rewritten using PHPUnit's internal tag checker (like was done at 8165137) then the failures would probably go away and we wouldn't have to rewrite them just because the way the fields are rendered is changed.

@phproberto
After applying the patch (setup: 3.3beta2, this patch with com_patchtester) get this error:
Fatal error: Cannot redeclare JFormField::validateRequirements() in /www/htdocs/custompath/libraries/joomla/form/field.php on line 1005
E.g. by accessing the menu item edit page at the Backend:
http://www.example.org/administrator/index.php?option=com_menus&view=items&menutype=usermenu
Or Mass Mail Users view:
http://www.example.org/administrator/index.php?option=com_users&view=mail

After turn this into a comment line The views will work.
https://github.com/phproberto/joomla-cms/blob/layouts-field/libraries/joomla/form/field.php#L1000-1024

@phproberto tests the debug feature is ok. Looks very good here, like it
Also User field look good here. (Tested at the com_content article view at the created_by option)
Also tested some backend views with some fields. I have not see that any one failed.
Good work here

Does anyone know what's happening with this PR?

Also, does this work with all form fields or just lists?

Sorry been meaning to reply to you on the mailing list! Basically we decided to expand the scope a bit and to implement it for the majority of form fields here: https://github.com/joomla-projects/cms-naked

However as you can probably see it lost popularity pretty quick. Me and @phproberto need to chat about what we're gonna do with it!

Oh okay, it's just that as it stands it's not possible to override the template (without a plugin) to accommodate any other CSS library that BS 2.3.

Would it be possible to just add layouts for the form fields? I can't see any b/c issues in doing this?

Can you point to any examples of where this has been done so that I can see the correct format / style for the overrides etc. I might be able to add them and put in a PR or two if I know what needs to be done.

Just use this PR as a base example with the rendering of the layouts. Or make a similar PR into the cms-naked branch and I have permissions there to merge that.

I completely agree by the way! I've hacked this into one of my sites to get integrations sorted in the frontend for the editing views.

Sorry @adster101 I should also jump into the issue. I was waiting for v3.5 but I think we have to start making that naked CMS.

I'll update the PR against the naked repo tonight and link it here so @wilsonge and me can use it as a starting point to also receive feedback.

Thanks!

What is 'naked' CMS? Is it just a place to add the layouts that will be merged into the staging branch at some point or something else?

@phproberto did you find time to update the repo?

Thanks.

Yeah so it's a place where we're gonna work on the next layer of making Joomla override-able. Firstly moving as much hardcoded stuff into JLayouts. But also thinking ahead a bit - adding in a getRenderer function (https://github.com/joomla-projects/cms-naked/blob/master/libraries/cms/plugin/plugin.php#L153 ) so that we can investigate not even being dependent on JLayouts.

The idea was that this would go into 3.5 (coming out ~now) but as you probably know 3.4 is being mega late so not quite sure where this is gonna fit in

Can you elaborate a bit on how the renderer would break the JLayout dependency? I mean what would the other rendering options be?

Is there any reason why this PR can't be merged with 3.4, when it's ready?

Can you elaborate a bit on how the renderer would break the JLayout dependency? I mean what would the other rendering options be?

Imho the longtime idea is that you could also use Twig or similar for rendering purposes. At least I understood it like this.

Is there any reason why this PR can't be merged with 3.4, when it's ready?

It could go into 3.4 in theory, but that would be something the release manager (currently for 3.4 that's David Hurley) has to decide. For 3.4, the main focus is a different one and since we're already delayed I wouldn't focus on this one. But if it gets ready in time, maybe it can be merged already.
On the other hand, the stuff in cms-naked is likely going to be a major focus for 3.5. So it would make sense to do this PR together with 3.5.

I am a bit play with new JLayoutFile and it really cool thing

Have couple suggestions:

1 Posibility to get fullpath without rendering the layout file, something like:

    public function getFullPath()
    {
        if (!is_null($this->fullPath))
        {
            return $this->fullPath;
        }
        // Search
        return $this->getPath();
    }

2 Throw Exception in render() if no layout file found. Currently if no layout found user just get a blank page, that very confused. It not a problem when need to render 1-2 layouts, but when there 100 different layouts can be difficult to find out.

3 What about ability use the priority in addIncludePath()?

Example, I want add own fallback path, so I do

   $layout->addIncludePath('/root/path/my/fallback/path');

And got:

  'includePaths' =>
    array (
      0 => string '/root/path/my/fallback/path'
      1 => string '/root/path/templates/isis/html/layouts' 
      2 => string '/root/path/layouts'
   )

That not really what I want, because I want that my fallback work only when no layout found in the template (or component) folder.
I think something like $layout->addIncludePath('/root/path/my/fallback/path', $priority); would be cool, but I have no idea how this can be done

added:
I think it can be as b/k solution, for the priority support:

public function addIncludePathToPosition($path, $position = 0)
{
    array_splice($this->includePaths, $position, 0, $path);
    return $this;
}

Hi,

please ckeck out review of the code of class JLayoutFile

1) Suggest don't run any debug function if debug not requested in options. I undestand that it makes a lot of "if ($this->debug)" but perfomance is more important then pretty code

2) In getPath method call "find" twice

$this->fullPath = JPath::find($this->includePaths, $rawPath);
if ($this->fullPath = JPath::find($this->includePaths, $rawPath))
{
   $this->addDebugMessage('<strong>Found layout:</strong> ' . $this->fullPath);
}
else
{
   $this->addDebugMessage('<strong>Unable to find layout: </strong> ' . $this->layoutId);
}

3) Method JLayoutFile::getPath() checks that file exists so you don't need to check it again in JLayoutFile ::render() method

4) Sugest also make cache on JLayoutHelper::render level from $options passed to render method and keep JLayout instances as value. It will work faster then JLayout cache. As a consequence JLayout's cache can store paths for LayoutId only , without md5 from all options. Some times it also very usefull when developer knows that state of JLayout whould not be changed, so we don't calculate every "render" md5 and json.

What is the decision on this one? If it is ready to commit I would like to bring the fields user, media and content history (versions) to layouts and use bootstrap modal (for protostar and isis) and thus drop completely the loading of mootools on joomla core...

The decision is that it is being held back for Joomla 3.5 - but the JLayout conversion will be centre feature of that release

Sounds good, thanks George

one more note,
I will leave here before forget
about Prefixes support

suggested search with prefixes is: template with prefix => component with prefix => template without prefix => component without prefix
it make possible case when template override will not work

so I think would be better make search like: template with prefix => template without prefix => component with prefix => component without prefix

@phproberto why did you close this one? There’s another approach/solution/code for this?