Review Request: Timer: one click to pause/resume, blinks when paused.

Aaron J. Seigo aseigo at kde.org
Wed Feb 2 07:40:03 CET 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100521/#review1151
-----------------------------------------------------------


good ideas; a suggestion for improvement: instead of just showing/hiding items, why not use a QPropertyAnimation connected to a Q_PROPERTY in the applet that sets the the opacity of the items so they fade in/out? it would probably look a lot more slick.


applets/timer/timer.h
<http://git.reviewboard.kde.org/r/100521/#comment963>

    watch the indentation :)



applets/timer/timer.cpp
<http://git.reviewboard.kde.org/r/100521/#comment964>

    you need to check if event->pos() is inside geometry()
    
    also, whitespace around curly braces :)



applets/timer/timer.cpp
<http://git.reviewboard.kde.org/r/100521/#comment966>

    instead of a jumble of letters like tsvgw, consider using something "plain" like "widget"



applets/timer/timer.cpp
<http://git.reviewboard.kde.org/r/100521/#comment967>

    must simpler as:
    
    widget->setVisible(!widget->isVisible());



applets/timer/timer.cpp
<http://git.reviewboard.kde.org/r/100521/#comment968>

    should be on same line


- Aaron J.


On Feb. 2, 2011, 4:25 a.m., Romário Rios wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100521/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2011, 4:25 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch does some little changes to Timer plasmoid:
>  - Now it takes only one click to start, resume and pause it
>  - It now blinks when paused, making it quicker to determine if it's paused or running
>  - Double-click resets timer
> 
> 
> Diffs
> -----
> 
>   applets/timer/timer.h d191fa3 
>   applets/timer/timer.cpp a09aa4c 
> 
> Diff: http://git.reviewboard.kde.org/r/100521/diff
> 
> 
> Testing
> -------
> 
> As it's a simple modification, I think that it brings no bugs (except by the fact that it says timeout when reseted with double-click).
> 
> 
> Thanks,
> 
> Romário
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110202/900a10bd/attachment-0001.htm 


More information about the Plasma-devel mailing list