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

2011-02-22 Thread Davide Bettio

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


Please, revert this patch or fix it:
* Currently single click doesn't seem to work
* I don't think double click for reset is a good idea.

- Davide


On Feb. 2, 2011, 9:39 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, 9:39 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
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


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

2011-02-02 Thread Romário Rios

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

(Updated Feb. 2, 2011, 9:34 a.m.)


Review request for Plasma.


Changes
---

Code correction, prettier blinking after some hours of messing up with 
intervals/easing curves/opacity values.


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 (updated)
-

  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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


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

2011-02-02 Thread Romário Rios

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

(Updated Feb. 2, 2011, 9:39 a.m.)


Review request for Plasma.


Changes
---

Apparently, I messed something up in the last diff.


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 (updated)
-

  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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


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

2011-02-02 Thread Aaron J. Seigo

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


do you have a commit account, or do you need someone to commit for you? reason 
i ask is that i've merged your patch here and made some adjustments to it; if 
you have a commit account i can send you an updated version of your patch to 
polish further if you wish and then push, otherwise, i can push it for you.

- Aaron J.


On Feb. 2, 2011, 9:39 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, 9:39 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
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


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

2011-02-02 Thread Romário Rios


 On Feb. 2, 2011, 4:31 p.m., Aaron J. Seigo wrote:
  do you have a commit account, or do you need someone to commit for you? 
  reason i ask is that i've merged your patch here and made some adjustments 
  to it; if you have a commit account i can send you an updated version of 
  your patch to polish further if you wish and then push, otherwise, i can 
  push it for you.

I have no commit account. Push it.


- Romário


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


On Feb. 2, 2011, 9:39 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, 9:39 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
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


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

2011-02-01 Thread Aaron J. Seigo

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


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel