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

Reply via email to