? Success

User tests: Successful: Unsuccessful:

avatar phproberto
phproberto
1 Nov 2015

This PR tries to improve & fix some issues in the curre JLayout system

Index

  1. Fixes
    1.1. [fix] Custom include paths cannot be used.
  2. Improvements
    2.1. [imp] Add method to clear include paths (clearIncludePaths())
    2.2. [imp] Add method to get active include paths (getIncludePaths())
    2.3. [imp] Make JLayout methods chainable when possible.
    2.4. [imp] Allow to set data variables before rendering the layout
    2,5. [imp] Add method to clear the debug messages (clearDebugMessages())
    2.6. [imp] Add a cache system to avoid duplicates file searches
    2.7. [imp] Add HTML comment to the start & end of layouts when Joomla debug mode is enabled
    2.8. [imp] Add support for automatic language suffixes
    2.9. [imp] Add support for automatic version suffixes
    2.10. [imp] Add a method to get the active layout id
    2.11. [imp] Add a method to get the active suffixes
    2.12. [imp] Add a method to check if debug is enabled
    2.13. [imp] Add fast methods to debug layouts
    2.14. [imp] Avoid adding debug messages if debug is disabled

1. Fixes

1.1. [fix] Custom include paths cannot be used.

Currently there is a fix that makes that includePaths cannot be changed by developers. See #3451

This was caused because the refreshIncludePaths() method was called always on render method. Not the logic attached to load the default include paths is only triggered when there are no includePaths explicilty defined.

Now you can use different methods to set the include paths:

$layout = new JLayoutFile('mylibrary.item');

// Explicitly set the array of include paths
$layout->setIncludePaths(array(JPATH_LIBRARIES . '/mylibrary/layouts'));

// Add a layout folder on top of the default includePaths
$layout->addIncludePath(JPATH_LIBRARIES . '/mylibrary/layouts');

// Add more than one folder on top of the default include paths
$layout->addIncludePaths(
    array(
        JPATH_LIBRARIES . '/mylibrary/layouts',
        JPATH_SITE . '/layouts'
    )
);

2. Improvements

2.1. [imp] Add method to clear include paths

Now there is a method to clear the list of include paths. Usage:

$layout = new JLayoutFile('mylibrary.item');

// Clear any existing include path
$layout->clearIncludePaths();

// Add our own paths
$layout->addIncludePaths(
    array(
        JPATH_LIBRARIES . '/mylibrary/layouts',
        JPATH_SITE . '/layouts'
    )
);

// Note that you don't need to clear include paths to replace them. This would be faster:
$layout->setIncludePaths(
    array(
        JPATH_LIBRARIES . '/mylibrary/layouts',
        JPATH_SITE . '/layouts'
    )
);

2.2. [imp] Add method to get active include paths

Now we can get the list of active paths that are going to be used to render a layout:

$layout = new JLayoutFile('mylibrary.item');

// Get the active include paths. If they are not set default include paths will be returned
$defaultPaths = $layout->getIncludePaths();

// Replace include paths
$layout->setIncludePaths(
    array(
        JPATH_LIBRARIES . '/mylibrary/layouts',
        JPATH_SITE . '/layouts'
    )
);

// Get the list of new paths assigned
$newPaths = $layout->getIncludePaths();

2.3. [imp] Make JLayout methods chainable when possible.

This allows a more dynamic way of setup layouts. Examples:

$layout = new JLayoutFile('mylibrary.item');

// Set our custom include paths
$layout->setIncludePaths(array(JPATH_LIBRARIES . '/mylibrary/layouts')
    // Enable debug
    ->setDebug(true)
    // Load our prefixes
    ->setSuffixes(array('bs3', 'bs2'));

// Render the layout
echo $layout->render(array('item' => $item));

2.4. [imp] Allow to set data variables before rendering the layout

Most rendering systems include a way to dynamically set & set data. Now you can do it like:

$layout = new JLayoutFile('mylibrary.item');

// Set a data value
$layout->set('name', 'Roberto');

// Sample of how it can be used
if ($useTooltips)
{
    $layout->set('tooltip', 'I hate tooltips');
}

// Check if a data value has been set. It will return the value if set or the default value if not set
// public function get($key, $defaultValue = null)
$age = (int) $layout->get('age', 0);

// You can still pass additional data on render and this data will be merged
$data = array(
    'sex' => 'almost never',
    'height' => 2.05
)

// Layout will receive: name, tooltip, sex & height values 
echo $layout->render($data);

Note that to be able to use the set & get methods the data passed to layouts has to be ALWAYS AN ARRAY. Otherwise the system will only use the data received in the render method to keep B/C.

2.5. [imp] Add method to clear the debug messages

Now debug messages can be cleared dynamically with $layout->clearDebugMessages(). Now that method is only used internally but may be useful even if it's only to people extending JLayoutFile

2.6. [imp] Add a cache system to avoid duplicates file searches

Now all the file searches are cached by the system so if the same layout is searched in the same include paths and with the same suffixes it will return a cached path.

2.7. [imp] Add HTML comment to the start & end of layouts when Joomla debug mode is enabled.

This was a proposal by @javigomez and I found it really useful for frontenders I've been working with. When debug mode is enabled from backend joomla configuration all the layouts will have an HTML comment before and one HTML after.

Something like:

<!-- Start layout: /home/roberto/www/jcms3x/layouts/joomla/content/info_block/block.php -->
<dl class="article-info muted">
    <dt class="article-info-term"> Details </dt>
</dl>
<!-- End layout: /home/roberto/www/jcms3x/layouts/joomla/content/info_block/block.php -->

2.8. [imp] Add support for automatic language suffixes

Now there is a method to automatically load suffixes based on the active language. Usage:

$layout = new JLayoutFile('joomla.content.info_block.block');

// Load the language suffixes
$layout->loadLanguageSuffixes();

// Render the layout
$layout->render(array());

/*
When en-GB language is enabled this will be the suffixes applied
Suffixes: Array
(
    [0] => en-GB
    [1] => en
    [2] => ltr
)
*/

/*
Sample file search:
Searching layout for: joomla/content/info_block/block.en-GB.php
Searching layout for: joomla/content/info_block/block.en.php
Searching layout for: joomla/content/info_block/block.ltr.php
Searching layout for: joomla/content/info_block/block.php
*/

2.9. [imp] Add support for automatic version suffixes

I added also a method to generate automatic suffixes based on the version of the system. Usage:

$layout = new JLayoutFile('joomla.content.info_block.block');

// Load the version suffixes
$layout->loadVersionSuffixes();

// Render the layout
$layout->render(array());

/*
Suffixes loaded for latest Joomla:
Suffixes: Array
(
    [0] => j350
    [1] => j35
    [2] => j3
)
*/

/*
Sample file search:
Searching layout for: joomla/content/info_block/block.j350.php
Searching layout for: joomla/content/info_block/block.j35.php
Searching layout for: joomla/content/info_block/block.j3.php
Searching layout for: joomla/content/info_block/block.php
*/

2.10. [imp] Add a method to get the active layout id

Sometimes is useful to know the id of the layout being rendered. I added a method to do it. Sample usage:

$layout = new JLayoutFile('joomla.content.info_block.block');

// Will print joomla.content.info_block.block
echo $layout->getLayoutId();

I have also deprecated the old setLayout() method in favour of setLayoutId() so describes better what it does.

2.11. [imp] Add a method to get the active suffixes

Returns an array of loaded suffixes. Sample usage:

$layout = new JLayoutFile('joomla.content.info_block.block');

// Load version suffixes
$layout->loadVersionSuffixes();

// Print the active suffixes
print_r($layout->getSuffixes());

/*
Array
(
    [0] => j350
    [1] => j35
    [2] => j3
)
*/

2.12. [imp] Add a method to check if debug is enabled

Now there is also a method to check if debug is enabled. It makes sense mainly for internal use. Sample usage:

public function addDebugMessage($message)
{
    if ($this->isDebugEnabled())
    {
        $this->debugMessages[] = $message;
    }

    return $this;
}

Also there is now a method to change debug mode without adding the debug setting to the options :

$layout = new JLayoutFile('joomla.content.info_block.block');

// Enable debug mode
$layout->setDebug(true);

echo $layout->render(array());

2.13. [imp] Add fast methods to debug layouts

Additionally there are new methods to fastly debug a layout.

$layout = new JLayoutFile('joomla.content.info_block.block');

// Standard rendering
echo $layout->render($data);

// Render with debug information
echo $layout->debug($data);

There is also a new method in the JLayoutHelper class:

echo JLayoutHelper::debug('joomla.content.info_block.block', $data);

So only replacing the render with debug you see the output with the debug info.

2.14. [imp] Avoid adding debug messages if debug is disabled

This has been removed because it causes tests to fail and there is no time before v3.5

This was a request from @alex-filat

Now the debug messages are only added and returned when debug is enabled.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar phproberto phproberto - open - 1 Nov 2015
avatar phproberto phproberto - change - 1 Nov 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 1 Nov 2015
Labels Added: ?
avatar phproberto phproberto - change - 1 Nov 2015
The description was changed
avatar phproberto
phproberto - comment - 1 Nov 2015

I'm going to remove the debug requirement to render debug messages because it doesn't really improve performance and causes tests to fail.

avatar wilsonge
wilsonge - comment - 1 Nov 2015

PR is currently failing because of downtime in pecl.php.net - as you can see the rest of the tests pass in other PHP versions - so assume they will pass in 5.5 and 5.6 for now and we will restart the build when it comes online

avatar wilsonge wilsonge - test_item - 2 Nov 2015 - Tested successfully
avatar wilsonge
wilsonge - comment - 2 Nov 2015

I have tested this item :white_check_mark: successfully on 825719f

Applied patch. Viewed some articles which used JLayouts by default to ensure that existing layouts continued to work as expected successfully.

Tested in my own module which currently is forced to override JLayoutFile to change the include paths (https://github.com/JoomJunk/shoutbox/blob/development/mod_shoutbox/libraries/layout.php) and tested replacing it with setIncludePaths and it worked successfully. I also tested doing version specific overrides here.

Enabled Debug mode and successfully observed the debug html comments - which are actually a great idea. Kunena use a similar concept which is super useful

Overall really useful and a successful test. Thanks @phproberto


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

avatar wilsonge wilsonge - test_item - 2 Nov 2015 - Tested successfully
avatar zero-24 zero-24 - test_item - 2 Nov 2015 - Tested successfully
avatar zero-24
zero-24 - comment - 2 Nov 2015

I have tested this item :white_check_mark: successfully on 825719f

Works great @phproberto I realy like the html Debug comments! :+1:

I have just send a quick CS PR here: phproberto#3 if we can get that in we can move this RTC.


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

avatar zero-24 zero-24 - change - 2 Nov 2015
Milestone Added:
avatar zero-24 zero-24 - change - 2 Nov 2015
Milestone Added:
avatar zero-24 zero-24 - change - 2 Nov 2015
Category Layout Libraries
avatar joomla-cms-bot
joomla-cms-bot - comment - 2 Nov 2015

This PR has received new commits.

CC: @wilsonge, @zero-24


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

avatar phproberto
phproberto - comment - 2 Nov 2015

Thanks for testing @wilsonge & @zero-24

I have merged your code style fixes.

Let's see if travis passes now!

avatar zero-24 zero-24 - change - 2 Nov 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 2 Nov 2015

Thanks @phproberto -> RTC


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

avatar joomla-cms-bot joomla-cms-bot - change - 2 Nov 2015
Labels Added: ?
avatar zero-24 zero-24 - alter_testresult - 2 Nov 2015 - wilsonge: Tested successfully
avatar zero-24 zero-24 - alter_testresult - 2 Nov 2015 - zero-24: Tested successfully
avatar roland-d roland-d - change - 2 Nov 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-11-02 11:47:42
Closed_By roland-d
avatar roland-d roland-d - close - 2 Nov 2015
avatar joomla-cms-bot joomla-cms-bot - close - 2 Nov 2015
avatar roland-d roland-d - reference | a16114c - 2 Nov 15
avatar roland-d roland-d - merge - 2 Nov 2015
avatar roland-d roland-d - close - 2 Nov 2015
avatar joomla-cms-bot joomla-cms-bot - change - 2 Nov 2015
Labels Removed: ?
avatar phproberto phproberto - head_ref_deleted - 2 Nov 2015

Add a Comment

Login with GitHub to post a comment