Review Request: Some improments to the system tray notifications.

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/256/#review251
-----------------------------------------------------------

Ship it!


- Aaron


On 2008-11-03 16:43:52, Rob Scheepmaker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/256/
> -----------------------------------------------------------
> 
> (Updated 2008-11-03 16:43:52)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> I noticed a couple of issues in the way system tray notifications are currently displayed:
> * The action buttons are hardcoded, and inconsistent with the rest of the buttons in plasma.
> Solution: use Plasma::PushButtons.
> * The close action should be a graphical 'X'.
> Solution: use the just added showCloseButton() to display an X in the drag handle of notifications that don't specify any actions.
> * The icon us quite large and takes up a lot of space.
> Solution: remove the icon from the notification itself and use it as the icon of the extenderitem.
> * The fixed size of the notifications make small messages take too much space, but also isn't enough to display larger messages.
> Solution: make the size hint depend on the size of the message. Make sure each notification is always displayed entirely.
> 
> While making these changes the current split in two classes became a bit awkward to work with, so I also merged these two classes into one QGW, and cleaned up a bit. I think that with this patch notifications are more usable, and the code is a bit cleaner, but I would very much like to hear your feedback.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/CMakeLists.txt
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/notificationwidget.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/notificationwidget.cpp
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/notifytextitem.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/notifytextitem.cpp
> 
> Diff: http://reviewboard.vidsolbach.de/r/256/diff
> 
> 
> Testing
> -------
> 
> seems to work allright.
> 
> 
> Screenshots
> -----------
> 
> notifications
>   http://reviewboard.vidsolbach.de/r/256/s/91/
> 
> 
> Thanks,
> 
> Rob
> 
>