User tests: Successful: Unsuccessful:
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
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
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.
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?
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.
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.
Labels |
Added:
?
?
|
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?
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
Please correct style (indentation). Otherwise works fine here.
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.
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&tracker_item_id=33197&start=0">http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33197&start=0</a></p> |
@infograf768 could you clarify, please?
Please decide if joomla.system.message
is the best path for this before merging. It will be a pain to change it later.
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?
By indentation I mean adding a tab for each line coming after an if or a foreach
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>
Title |
|
Awesome :) As that was just code style I think we can merge this now JM :)
Sorry it was holidays here! :)
Great addition. joomla.system.message
looks ok here
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-03-20 16:14:35 |
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?