Review Request: Some reworking of the job widget used for the systemtray.

[Aaron Seigo]

-----------------------------------------------------------
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
> 
>