<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/110327/">http://git.reviewboard.kde.org/r/110327/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 7th, 2013, 12:50 p.m. UTC, <b>Dominik Haumann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The patch itself is fine and most likely does not introduce regressions in terms of misbehavior.
Still, is never showing an icon the way to go? Another way to work around this by default would be an additional function called KMessageWidget::setShowIcon(bool).
In fact, I've recently been thinking that being able to set custom icons also may be a good idea, along with setting a custom color for the message widget. Maybe one could ex extend MessageType, i.e.: setMessageType(Custom). Along with setIcon() and setPalette() or similar. Then, the developer would have more control over the colors showing up in the Kate views. A disadvantage of this is, however, that this leads to inconsistent ui's.
It this is needed, this patch works against this, though.</pre>
</blockquote>
<p>On May 7th, 2013, 5:03 p.m. UTC, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It could be nice to be able to use a custom icon which would be adapted to the context of the message. But I think the widget still works without it for now, and getting rid of it has its advantages (fixing confusion, saving space). An alternative to this fix would be to replace the icon with something less button-like. The only icon I think would work here is the "dialog-warning" one, which is already used with KMessageWidget::Warning type. Any other idea?
Exposing access to the palette would indeed be dangerous for consistency. Can you explain in which situation you would want control over the colors?</pre>
</blockquote>
<p>On May 7th, 2013, 5:09 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What about making the icon a "watermark"?</pre>
</blockquote>
<p>On May 7th, 2013, 7:44 p.m. UTC, <b>Dominik Haumann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Using icons as watermark was already discussed years ago. I didn't read the entire thread, but on the following link you can find a url to an image showing a watermark icon in the background. The thread is very long, and it the end nothing came out of it apparently:
http://lists.kde.org/?l=kde-core-devel&m=120204190217471&w=2</pre>
</blockquote>
<p>On May 7th, 2013, 7:47 p.m. UTC, <b>Dominik Haumann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> Exposing access to the palette would indeed be dangerous for consistency. Can you explain in which situation you would want control over the colors?
Not really: We just had once someone on kwrite-devel (iirc) complaining about the colors. But if you ask me, the colors are fine for the use cases right now.</pre>
</blockquote>
<p>On May 9th, 2013, 12:03 p.m. UTC, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't like watermarks, they look too noisy. Having thought about this patch more, I would like to keep the ability to set the icon. What about removing all icons by default but adding an "icon" property?
This would fix the confusion for KMessageWidget::Error and still saves space while giving the ability to set icons more adapted to the widget message when there is a need for it.</pre>
</blockquote>
<p>On May 10th, 2013, 8:57 a.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think we also need a neutral message type that uses the window/button background color (as a gradient, of course) as widget background for displaying things that aren't really an "information" but rather "progress" such as the Kate "document is still loading" message.
And for Kate I could think of many cases where, instead of using the generic warning icon or no icon at all, using a more specific one could improve "first-sight-recognizability" such as 'object-locked' for when the file has been opened read-only, or 'character-set' when there's encoding issues.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> as a gradient, of course
http://api.kde.org/4.x-api/kdelibs-apidocs/kdeui/html/classKStyle.html#a679a9290cff89d0f2e330cd448216d2d
Reg. the "icon not prominent enough" that's easily solved by, as Kai suggested, using a sensible icon in the first place (eg. one that does not look like a close button, to start with) and then let it occupy the entire left or right half of the widget, resp. cap that at (dpi adjusted) 128px.
Then *partially* overlay it with a translucent bg colored rect and put the text in there.
The imporant aspect of all those actions should be to make it look like not-a-button.
If you can't imagine what i've in mind, i'll sketch a mock this evening.
</pre>
<br />
<p>- Thomas</p>
<br />
<p>On May 6th, 2013, 8:59 p.m. UTC, Aurélien Gâteau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Dolphin, Kate and kdelibs.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated May 6, 2013, 8:59 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This avoids confusion between the decoration icon and the close button, especially when type is KMessageWidget::Error. This happens for example with Dolphin when an error happens while trying to connect to an non available host.
This change also has the nice side-effect of leaving more space for the widget text.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested with kmessagewidgetdemo, Dolphin and Kate.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdeui/widgets/kmessagewidget.cpp <span style="color: grey">(a52316726233a22929ce8ad3aff60b9ccc5f9b85)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110327/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>