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