? Success

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000
okonomiyaki3000
28 Jan 2014

Let's render system messages with layouts for convenient overriding and simpler code.

Also, tests.
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33197&start=0

avatar okonomiyaki3000 okonomiyaki3000 - open - 28 Jan 2014
avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Jan 2014

I wonder where is the best place to put this layout. I've put it in joomla.system.message but there could be a better location for it. Any ideas?

avatar Bakual
Bakual - comment - 28 Jan 2014

I think there is already a way to override the system messages. Beez uses this: https://github.com/joomla/joomla-cms/blob/staging/templates/beez3/html/message.php

avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Jan 2014

Of course that's correct. I'm proposing this as a replacement for that (although the old-school method still works) because layouts should be the preferred way to override things like this. I intend to do the same for pagination.

avatar Bakual
Bakual - comment - 28 Jan 2014

Ah ok. Should we then deprecate the existing way? To me it doesn't make much sense to have multiple ways to achieve the same thing. Or does the old way allow for something which the new way doesn't?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Jan 2014

Yes, we should deprecate the old way, there is no advantage to using it. And once it's removed, a lot of the code for JDocumentRendererMessage can be cut out. Basically, the render function doesn't need to do anything except get a list of messages, pass it to the layout renderer, and return the result.

avatar Bakual
Bakual - comment - 28 Jan 2014

Can you add a deprecation message using JLog and as a comment? So it could be removed with 4.0. That would be great.
Good idea by the way on this PR.

avatar okonomiyaki3000 okonomiyaki3000 - change - 29 Jan 2014
Labels Added: ? ?
avatar okonomiyaki3000
okonomiyaki3000 - comment - 29 Jan 2014

Added the log. Also, restructured the function a little, the code that can be removed is everything from $app through the second if. And I'm passing the function arguments into the layout as well because... why not?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 29 Jan 2014

Oh, and I linked the PR from JC yesterday but forgot to link back to it from here.
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33197&start=0

avatar infograf768
infograf768 - comment - 29 Jan 2014

Please correct style (indentation). Otherwise works fine here.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 29 Jan 2014

I'm not sure what you mean about indentation. Are you talking about the layout file or message.php? The layout file doesn't pass phpcs but message.php does.

avatar okonomiyaki3000 okonomiyaki3000 - change - 30 Jan 2014
The description was changed
Description <p>Let's render system messages with layouts for convenient overriding and simpler code.</p> <p>Also, tests.</p> <p>Let's render system messages with layouts for convenient overriding and simpler code.</p> <p>Also, tests.<br><a href="http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&amp;tracker_item_id=33197&amp;start=0">http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&amp;tracker_item_id=33197&amp;start=0</a></p>
avatar okonomiyaki3000
okonomiyaki3000 - comment - 1 Feb 2014

@infograf768 could you clarify, please?

avatar wilsonge
wilsonge - comment - 11 Mar 2014

@test works as expected. RTC

avatar okonomiyaki3000
okonomiyaki3000 - comment - 11 Mar 2014

Please decide if joomla.system.message is the best path for this before merging. It will be a pain to change it later.

avatar Bakual
Bakual - comment - 11 Mar 2014

Please decide if joomla.system.message is the best path for this before merging. It will be a pain to change it later.

@phproberto What do you think?

avatar infograf768
infograf768 - comment - 12 Mar 2014

@okonomiyaki3000

By indentation I mean adding a tab for each line coming after an if or a foreach

avatar infograf768
infograf768 - comment - 16 Mar 2014

I mean something like this:

<?php
/**
 * @package     Joomla.Site
 * @subpackage  Layout
 *
 * @copyright   Copyright (C) 2005 - 2014 Open Source Matters, Inc. All rights reserved.
 * @license     GNU General Public License version 2 or later; see LICENSE.txt
 */

defined('JPATH_BASE') or die;

$msgList = $displayData['msgList'];

?>
<div id="system-message-container">
    <?php if (is_array($msgList)) : ?>
        <div id="system-message">
            <?php foreach ($msgList as $type => $msgs) : ?>
                <div class="alert alert-<?php echo $type; ?>">
                    <?php // This requires JS so we should add it trough JS. Progressive enhancement and stuff. ?>
                    <a class="close" data-dismiss="alert">×</a>

                    <?php if (!empty($msgs)) : ?>
                        <h4 class="alert-heading"><?php echo JText::_($type); ?></h4>
                        <div>
                            <?php foreach ($msgs as $msg) : ?>
                                <p><?php echo $msg; ?></p>
                            <?php endforeach; ?>
                        </div>
                    <?php endif; ?>
                </div>
            <?php endforeach; ?>
        </div>
    <?php endif; ?>
</div>
avatar okonomiyaki3000 okonomiyaki3000 - change - 16 Mar 2014
Title
Message as layout
[#33197] Message as layout
avatar wilsonge
wilsonge - comment - 16 Mar 2014

Awesome :) As that was just code style I think we can merge this now JM :)

avatar phproberto
phproberto - comment - 20 Mar 2014

Sorry it was holidays here! :)

Great addition. joomla.system.message looks ok here

avatar phproberto phproberto - reference | 74a70d4 - 20 Mar 14
avatar phproberto phproberto - change - 20 Mar 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-03-20 16:14:35
avatar phproberto phproberto - close - 20 Mar 2014
avatar phproberto phproberto - close - 20 Mar 2014
avatar okonomiyaki3000 okonomiyaki3000 - head_ref_deleted - 24 Mar 2014
avatar puneet0191 puneet0191 - reference | 9926c24 - 30 Mar 14
avatar Bakual Bakual - reference | 443b00c - 12 May 14

Add a Comment

Login with GitHub to post a comment