----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/539/#review880 -----------------------------------------------------------
Ship it! the magic numbers scare me slightly, but that's easily fixed :) /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/jobwidget.cpp <http://reviewboard.kde.org/r/539/#comment551> magic numbers (e.g. 7) are brittle; can these be replaced with static const int's? - Aaron On 2009-04-08 17:19:21, Rob Scheepmaker wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/539/ > ----------------------------------------------------------- > > (Updated 2009-04-08 17:19:21) > > > Review request for Plasma. > > > Summary > ------- > > There are some problems with the current job widget: > * It takes up a lot of vertical space (6 lines of text + progress bar) > * It looks a bit chaotic > * It doesn't include an ETA > > This patch tries to solve these issues. We display an ETA and speed always > (imo those are the most relevant pieces of information), and hide the rest > under a 'more' link. Also we display a total ETA in the groupwidget's title. > See the attached screenshot. > Some things could still be improved... like only taking up room for labels > that actually contain text in the 'more' part. But I think this is already a > big improvement. Ok to commit? > > > Diffs > ----- > > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/job.h 950703 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/job.cpp 950703 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/core/manager.cpp > 951176 > > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/jobs/dbusjobprotocol.cpp > 950703 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/jobwidget.h > 950703 > /trunk/KDE/kdebase/workspace/plasma/applets/systemtray/ui/jobwidget.cpp > 950703 > > /trunk/KDE/kdebase/workspace/plasma/dataengines/applicationjobs/kuiserverengine.cpp > 950703 > > Diff: http://reviewboard.kde.org/r/539/diff > > > Testing > ------- > > > Screenshots > ----------- > > > http://reviewboard.kde.org/r/539/s/103/ > > > Thanks, > > Rob > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel