-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110327/#review32194
-----------------------------------------------------------


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.

- Dominik Haumann


On May 6, 2013, 8:59 p.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110327/
> -----------------------------------------------------------
> 
> (Updated May 6, 2013, 8:59 p.m.)
> 
> 
> Review request for Dolphin, Kate and kdelibs.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kdeui/widgets/kmessagewidget.cpp a52316726233a22929ce8ad3aff60b9ccc5f9b85 
> 
> Diff: http://git.reviewboard.kde.org/r/110327/diff/
> 
> 
> Testing
> -------
> 
> Tested with kmessagewidgetdemo, Dolphin and Kate.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

Reply via email to