----------------------------------------------------------- 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 > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
