Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 3, 2014, 3:21 p.m., Commit Hook wrote:
> > This review has been submitted with commit 
> > a76d23770ae075c55d0b3cc063cc1a073c639ad6 by Shantanu Tushar to branch 
> > master.

It is pleasure to made some contribution to the KDE. Also i hope to Do more to 
the plasma-Media center.
Thanks you very much hantanu Tushar and Sinny Kumari for guiding me.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51802
---


On March 3, 2014, 3:21 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 3, 2014, 3:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Commit Hook]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51802
---


This review has been submitted with commit 
a76d23770ae075c55d0b3cc063cc1a073c639ad6 by Shantanu Tushar to branch master.

- Commit Hook


On March 3, 2014, 3:13 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 3, 2014, 3:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 3, 2014, 3:21 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51801
---

Ship it!


Ship It!

- Shantanu Tushar


On March 3, 2014, 3:13 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 3, 2014, 3:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 3, 2014, 3:07 p.m., Shantanu Tushar wrote:
> > mediaelements/mediacontroller/MediaController.qml, lines 201-205
> > 
> >
> > why are you doing it like this now? This previous code was better-
> > 
> > onReleased: volumeButton.checked = !volumeButton.checked

yes. 
But with this "onReleased: volumeButton.checked = !volumeButton.checked" i 
toggle the button state.
that why rechange to this way. i fix this.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51799
---


On March 3, 2014, 3:13 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 3, 2014, 3:13 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 3, 2014, 3:13 p.m.)


Review request for Plasma.


Changes
---

Fixed to the "volumeButton.checked = !volumeButton.checked"


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51799
---



mediaelements/mediacontroller/MediaController.qml


why are you doing it like this now? This previous code was better-

onReleased: volumeButton.checked = !volumeButton.checked


- Shantanu Tushar


On March 3, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 3, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 3, 2014, 3:03 p.m.)


Review request for Plasma.


Changes
---

replace mousePressed event with mouseRelase event.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 2, 2014, 11:16 a.m., Shantanu Tushar wrote:
> > Good thinking, and in fact this is the idea that first came to my mind. 
> > But, as you would've noticed, the experience on clicking the button isn't 
> > very natural now because you aren't really pressing the button.
> > Another (and better, imo) solution you can try is reverse the parenting 
> > order of MouseEventListener and ToolButton (i.e. MouseEventListener { 
> > ToolButton { } }). This way you will get both the interactions (click and 
> > scroll) and will still feel natural.
> 
> Rukshan Chathuranga wrote:
> Oh really. But i can't see any diffent or any unnatural behavior after 
> this modification. i'm sorry, may be i can't see that small difference. But 
> my suggestion is until we find the most appropriate way, we can use this 
> modification as the fixed one. 
>
> 
> Shantanu Tushar wrote:
> Without the patch, the volume button reacts to your pushing and releasing 
> the mouse button on the volume button. With the patch, it works even if you 
> don't release the mouse button (that is one of the things), etc.
> As I said, all you need to do to fix this is to change the order of 
> parenting. If you get any problems, let me know, in that case I can do it for 
> you :)
> 
> Rukshan Chathuranga wrote:
> o thanks Tushar, i will work on that and let you know any trouble if 
> i got.

Hi Tushar, I understood the problem with the above patch. So i got a solution 
for that.

Before i used the mousePressed() event. Now can i used the MouseRelease() 
event, now it worked fine as before patch.
And also i think that this will solve the whole problem.
But if there are any issue let me know.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51601
---


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 2, 2014, 11:16 a.m., Shantanu Tushar wrote:
> > Good thinking, and in fact this is the idea that first came to my mind. 
> > But, as you would've noticed, the experience on clicking the button isn't 
> > very natural now because you aren't really pressing the button.
> > Another (and better, imo) solution you can try is reverse the parenting 
> > order of MouseEventListener and ToolButton (i.e. MouseEventListener { 
> > ToolButton { } }). This way you will get both the interactions (click and 
> > scroll) and will still feel natural.
> 
> Rukshan Chathuranga wrote:
> Oh really. But i can't see any diffent or any unnatural behavior after 
> this modification. i'm sorry, may be i can't see that small difference. But 
> my suggestion is until we find the most appropriate way, we can use this 
> modification as the fixed one. 
>
> 
> Shantanu Tushar wrote:
> Without the patch, the volume button reacts to your pushing and releasing 
> the mouse button on the volume button. With the patch, it works even if you 
> don't release the mouse button (that is one of the things), etc.
> As I said, all you need to do to fix this is to change the order of 
> parenting. If you get any problems, let me know, in that case I can do it for 
> you :)

o thanks Tushar, i will work on that and let you know any trouble if i got.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51601
---


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

> On March 2, 2014, 11:16 a.m., Shantanu Tushar wrote:
> > Good thinking, and in fact this is the idea that first came to my mind. 
> > But, as you would've noticed, the experience on clicking the button isn't 
> > very natural now because you aren't really pressing the button.
> > Another (and better, imo) solution you can try is reverse the parenting 
> > order of MouseEventListener and ToolButton (i.e. MouseEventListener { 
> > ToolButton { } }). This way you will get both the interactions (click and 
> > scroll) and will still feel natural.
> 
> Rukshan Chathuranga wrote:
> Oh really. But i can't see any diffent or any unnatural behavior after 
> this modification. i'm sorry, may be i can't see that small difference. But 
> my suggestion is until we find the most appropriate way, we can use this 
> modification as the fixed one. 
>

Without the patch, the volume button reacts to your pushing and releasing the 
mouse button on the volume button. With the patch, it works even if you don't 
release the mouse button (that is one of the things), etc.
As I said, all you need to do to fix this is to change the order of parenting. 
If you get any problems, let me know, in that case I can do it for you :)


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51601
---


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 2, 2014, 11:16 a.m., Shantanu Tushar wrote:
> > Good thinking, and in fact this is the idea that first came to my mind. 
> > But, as you would've noticed, the experience on clicking the button isn't 
> > very natural now because you aren't really pressing the button.
> > Another (and better, imo) solution you can try is reverse the parenting 
> > order of MouseEventListener and ToolButton (i.e. MouseEventListener { 
> > ToolButton { } }). This way you will get both the interactions (click and 
> > scroll) and will still feel natural.

Oh really. But i can't see any diffent or any unnatural behavior after this 
modification. i'm sorry, may be i can't see that small difference. But my 
suggestion is until we find the most appropriate way, we can use this 
modification as the fixed one. 


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51601
---


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51601
---


Good thinking, and in fact this is the idea that first came to my mind. But, as 
you would've noticed, the experience on clicking the button isn't very natural 
now because you aren't really pressing the button.
Another (and better, imo) solution you can try is reverse the parenting order 
of MouseEventListener and ToolButton (i.e. MouseEventListener { ToolButton { } 
}). This way you will get both the interactions (click and scroll) and will 
still feel natural.

- Shantanu Tushar


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 1, 2014, 12:57 p.m., Shantanu Tushar wrote:
> > I've got good news and bad news. The good news is, the wheel works perfect!
> > 
> > The bad news is that the MouseEventListener eats up clicks and hence the 
> > volumeButton never gets clicked which then means that you won't be able to 
> > show the volume slider. Hint to the solution lies in the order in which Qt 
> > delivers events (like mouse event in this case). Try if you can figure it 
> > out :)
> 
> Rukshan Chathuranga wrote:
> ohhh. i will try to fix that also.

hi, 
i fixed the volume icon click event when working with wheel event.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51430
---


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 1, 2014, 12:57 p.m., Shantanu Tushar wrote:
> > I've got good news and bad news. The good news is, the wheel works perfect!
> > 
> > The bad news is that the MouseEventListener eats up clicks and hence the 
> > volumeButton never gets clicked which then means that you won't be able to 
> > show the volume slider. Hint to the solution lies in the order in which Qt 
> > delivers events (like mouse event in this case). Try if you can figure it 
> > out :)

ohhh. i will try to fix that also.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51430
---


On March 1, 2014, 3:03 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 1, 2014, 3:03 p.m.)


Review request for Plasma.


Changes
---

fix the volume icon click when working with wheel event.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51430
---


I've got good news and bad news. The good news is, the wheel works perfect!

The bad news is that the MouseEventListener eats up clicks and hence the 
volumeButton never gets clicked which then means that you won't be able to show 
the volume slider. Hint to the solution lies in the order in which Qt delivers 
events (like mouse event in this case). Try if you can figure it out :)

- Shantanu Tushar


On March 1, 2014, 12:42 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 12:42 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 1, 2014, 12:15 p.m., Shantanu Tushar wrote:
> > mediaelements/mediacontroller/MediaController.qml, lines 157-169
> > 
> >
> > Please configure your editor to use 4 spaces instead of tabs. The next 
> > diff you upload should be spaces instead of tabs.
> > For more, read http://techbase.kde.org/Policies/Kdelibs_Coding_Style

Thans you for your help. i fixed it as you mention. 


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51423
---


On March 1, 2014, 12:42 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 12:42 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 1, 2014, 12:42 p.m.)


Review request for Plasma.


Changes
---

fix space in pcm before volume button. move code and fixed for mouse wheel 
event.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51423
---



mediaelements/mediacontroller/MediaController.qml


This creates an empty space between the curMediaTime Label and volumeSlider 
as these items (Including the MouseEventListener) are in a Row {}.
The right thing to do is to move the MouseEventListener inside the 
volumeButton and anchor.fill: parent



mediaelements/mediacontroller/MediaController.qml


Please configure your editor to use 4 spaces instead of tabs. The next diff 
you upload should be spaces instead of tabs.
For more, read http://techbase.kde.org/Policies/Kdelibs_Coding_Style


- Shantanu Tushar


On March 1, 2014, 11:32 a.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 11:32 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 1, 2014, 11:32 a.m.)


Review request for Plasma.


Changes
---

Removed White sapces.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Bhushan Shah]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51416
---


The whitespace is still there, if you see diff here on reviewboard you will 
realize that. L167 and L193.

- Bhushan Shah


On March 1, 2014, 3:49 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 3:49 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 1, 2014, 10:19 a.m.)


Review request for Plasma.


Changes
---

Remove unwanted imports and remove whitespaces.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Bhushan Shah]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51408
---



mediaelements/mediacontroller/MediaController.qml


PlasmaCore is already included.. ^



mediaelements/mediacontroller/MediaController.qml


Remove White space here and at other parts. For future if you are using 
Kate,

Set Settings > Configure Kate > Open/Save > Remove trailing spaces to on 
modified lines.



mediaelements/mediacontroller/MediaController.qml


whitespace


- Bhushan Shah


On March 1, 2014, 12:57 p.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 12:57 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

(Updated March 1, 2014, 7:27 a.m.)


Review request for Plasma.


Changes
---

updated diff file for MediaController.qml.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs (updated)
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 1, 2014, 7:16 a.m., Shantanu Tushar wrote:
> > Your diff contains commented out changes to metadataupdater.cpp which I 
> > think you don't need. Please revert changes to that file and post a fresh 
> > diff.
> > 
> > (Also, you even copied "(END)" which mostly happened while copy-pasting 
> > from the terminal, use "git diff > patchname.diff" which will save the 
> > exact diff to patchname.diff which you can then upload.)
> 
> Rukshan Chathuranga wrote:
> sorry for that.
> But i have chnged some things in the metadataupdater.cpp to support my 
> ubuntu version.
> So i cant change those. what should i do?
> 
> Shantanu Tushar wrote:
> ah, you can just use git diff  to show diff only for a 
> particular file (the qml one in your case)

ok. Thanks. I upload the new version.


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51404
---


On March 1, 2014, 7:27 a.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 7:27 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

> On March 1, 2014, 7:16 a.m., Shantanu Tushar wrote:
> > Your diff contains commented out changes to metadataupdater.cpp which I 
> > think you don't need. Please revert changes to that file and post a fresh 
> > diff.
> > 
> > (Also, you even copied "(END)" which mostly happened while copy-pasting 
> > from the terminal, use "git diff > patchname.diff" which will save the 
> > exact diff to patchname.diff which you can then upload.)
> 
> Rukshan Chathuranga wrote:
> sorry for that.
> But i have chnged some things in the metadataupdater.cpp to support my 
> ubuntu version.
> So i cant change those. what should i do?

ah, you can just use git diff  to show diff only for a particular 
file (the qml one in your case)


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51404
---


On March 1, 2014, 7:11 a.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 7:11 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

> On March 1, 2014, 7:16 a.m., Shantanu Tushar wrote:
> > Your diff contains commented out changes to metadataupdater.cpp which I 
> > think you don't need. Please revert changes to that file and post a fresh 
> > diff.
> > 
> > (Also, you even copied "(END)" which mostly happened while copy-pasting 
> > from the terminal, use "git diff > patchname.diff" which will save the 
> > exact diff to patchname.diff which you can then upload.)

sorry for that.
But i have chnged some things in the metadataupdater.cpp to support my ubuntu 
version.
So i cant change those. what should i do?


- Rukshan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51404
---


On March 1, 2014, 7:11 a.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 7:11 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Re: Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Shantanu Tushar]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116517/#review51404
---


Your diff contains commented out changes to metadataupdater.cpp which I think 
you don't need. Please revert changes to that file and post a fresh diff.

(Also, you even copied "(END)" which mostly happened while copy-pasting from 
the terminal, use "git diff > patchname.diff" which will save the exact diff to 
patchname.diff which you can then upload.)

- Shantanu Tushar


On March 1, 2014, 7:11 a.m., Rukshan Chathuranga wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116517/
> ---
> 
> (Updated March 1, 2014, 7:11 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implement change volume on mouse wheel when top of the voulume icon.
> 
> 
> Diffs
> -
> 
>   mediaelements/mediacontroller/MediaController.qml 968c459 
> 
> Diff: https://git.reviewboard.kde.org/r/116517/diff/
> 
> 
> Testing
> ---
> 
> tested the function and not change the default controlling due to new changes.
> 
> 
> Thanks,
> 
> Rukshan Chathuranga
> 
>

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

Review Request 116517: Implement change volume on mouse wheel when top of the voulume icon. based on the "Bug 322821 - Make volume controllable using mouse scroll "

[Rukshan Chathuranga]

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

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

Implement change volume on mouse wheel when top of the voulume icon.


Diffs
-

  mediaelements/mediacontroller/MediaController.qml 968c459 

Diff: https://git.reviewboard.kde.org/r/116517/diff/


Testing
---

tested the function and not change the default controlling due to new changes.


Thanks,

Rukshan Chathuranga

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