[TYPO3-core] RFC #11684: Add support for flash messages in the backend

Ingo Renner ingo at typo3.org
Wed Aug 19 12:17:54 CEST 2009


Francois Suter wrote:

Hi Francois,

> I'm attaching here a modified patch. I did 2 changes essentially:
>
> - I think the rendering of the messages should include a bit more
> markup, as it will allow for better skinning. I have change the
> rendering to match the one originally proposed in the Usability team
> discussion.

I'm nearly fine with that change. Some remarks though:
If we take header and body in accordance to an HTML document it should 
be like the following, and would also remove one unnecessary level of a div:

$title = '';
if (!empty($this->title)) {
	$title = '<div class="message-header">'
	. $this->title
	. '</div>';
}

$message = '<div class="typo3-message message-'
	. $classes[$this->severity] . '">'
	. $title
	. '<div class="message-body">' . $this->message . '</div>'
	. '</div>';

Thus typo3-message would be the <html> document, and the message-header 
would sit on top of the message-body

> - we discussed offline about the possibility of tagging messages, so
> that there's not a single queue but actually several. We agreed then
> that this could be postponed. However looking at the code you submitted,
> I found it very easy to add such feature, by adding an optional
> parameter to the queue management methods (pop, push, render and
> getFromSession). The tagging is optional. All message go to a default
> queue and that default queue still gets displayed at the top of a BE
> module's content. But it is also possible to call the
> template::renderFlashMessages() method manually (changed its visibility
> to public) in one's code to display another set of messages. I'm
> including a modified version of the test extension where you can see
> this in action.

I see your point and thus checked your changes. First I'd like to call 
it type instead of tag or context, type is more appropriate here.
But then your implementation is faulty: We never know about all existing 
types and thus it doesn't allow to retrieve and show all available 
messages at once.

I checked with Drupal's implementation (as I had it laying around here 
anyways) and found that they support types. What they can do is to 
display either all messages or just a specific type. I guess that's also 
what you wanted to achieve.

The soultion is to store the types differently then you did. We have to 
make types a sub-array of the overall messages array, thus we can still 
retrieve all types and also request specific types. Right now your 
implementation makes the type part of the key under which messages are 
stored. But to retrieve these messages you have to know about all 
available types, otherwise you'd have to guess...

So I think that types are a nice addition to the messages, but I'd like 
to introduce them as a separate feature/RFC as types would require some 
additional testing. I'd like to focus on getting flash messages in at 
all for now. Hope this is ok with you.

>> The icons need to be placed in typo3/gfx/. The notice message
>> intentionally doesn't have an icon as it's got very low severity.
>
> I still wonder about this. It looks really empty without an icon. I
> would prefer if it had one.

Well, if you can find an icon that fits the rest of the set, fine with 
me. In general I wouldn't care about that though as it's probably a very 
seldome message anyways.

> I don't really see a need to call htmlspecialchars() when instantiating
> a message. If a developer has doubts about the content of a message
> (because he repeats an input string, for example), he should call
> htmlspecialchars() first inside his code, before using that string
> inside a message.

sounds good to me.

To sum everything up:
* HTML rendering accepted with some minor changes.
* Types accepted, but as a separate RFC
* I don't care for an icon for notice messages. I am fine if you can 
find one that fits the rest (silk doesn't offer one!), but I wouldn't 
like to have this little nagger block the introduction of flash messages.
* Thanks for the feedback concerning htmlspecialchars(), I'm with your 
opinion.


all the best
Ingo

-- 
Ingo Renner
TYPO3 Core Developer, Release Manager TYPO3 4.2



More information about the TYPO3-team-core mailing list