subject:"Review Request 112208\: KMix qml applet"

On Sunday, September 01, 2013 10:27:27 Diego Casella wrote:
 @sebas ping?

It's on my list, sorry for the delay.
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 112208: KMix qml applet

2013-09-02 Thread Diego Casella



 On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
  http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55
 
  if volume is changed from kmix, not from the applet, this would issue a 
  duplicate service call to dataengine.
  
  my solution is to add a bool protector to disable this signal if the 
  data change is not from user.
 
 Xuetian Weng wrote:
 sorry for noise, maybe you should try onSliderMoved signal instead.
 
 Diego Casella wrote:
 cannot bind to a non-existent property, says plasma ... are you sure this 
 method exists?
 
 Xuetian Weng wrote:
 ah sorry... I initially thought it's Plasma/Slider in cpp but now it 
 seems it's another implementation in qml.
 the cpp version of Plasma/Slider is actually in plasma.graphicswidget but 
 since we're moving to qt5 in the future graphicswidget implementation is not 
 a preferable solution...
 
 So you might want to use the solution in my first comment.

imho it is too hackish and the applet works good even in this state. Do we 
really need to introduce something like that when you change the volume levels 
for, say, a total of 5 minutes every day (overstimated)?


- Diego


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

2013-09-02 Thread Xuetian Weng



 On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
  http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55
 
  if volume is changed from kmix, not from the applet, this would issue a 
  duplicate service call to dataengine.
  
  my solution is to add a bool protector to disable this signal if the 
  data change is not from user.
 
 Xuetian Weng wrote:
 sorry for noise, maybe you should try onSliderMoved signal instead.
 
 Diego Casella wrote:
 cannot bind to a non-existent property, says plasma ... are you sure this 
 method exists?
 
 Xuetian Weng wrote:
 ah sorry... I initially thought it's Plasma/Slider in cpp but now it 
 seems it's another implementation in qml.
 the cpp version of Plasma/Slider is actually in plasma.graphicswidget but 
 since we're moving to qt5 in the future graphicswidget implementation is not 
 a preferable solution...
 
 So you might want to use the solution in my first comment.
 
 Diego Casella wrote:
 imho it is too hackish and the applet works good even in this state. Do 
 we really need to introduce something like that when you change the volume 
 levels for, say, a total of 5 minutes every day (overstimated)?

There is some possible data race when last time I looked at the brightness 
slider in battery plasmoid. Especially when user drag the slider which 
generates more than one onValueChanged (Click on slider is usually ok).

And the slider looks very jumpy in that case.

user drags the slider 
onValueChanged caused by user
call setVolume to A to backend
user drags slider further to B
onValueChanged caused by user
call setVolume B to backend

backend notifies the change of A
onValueChanged caused by backend notificatoin
slider change back to A but user mouse is actually already on B.
and now it will send setVolume to A again.
and it will generate another onValueChanged.

Eventually it will have volume at B but if the difference of value A and B is 
huge, the visual reflection would be really bad.
And in some case it might generate really quite a lot onValueChanged (since 
theoretically it can generate infinite ping-pong loop if all events arrives 
just-in-time.). 

I didn't test your plasmoid but you can try to drag it aggressively from on 
side to the other side to see if this is really a issue.


- Xuetian


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and

Re: Review Request 112208: KMix qml applet

2013-09-02 Thread Diego Casella

On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55

if volume is changed from kmix, not from the applet, this would issue a
duplicate service call to dataengine.

my solution is to add a bool protector to disable this signal if the
data change is not from user.

Xuetian Weng wrote:
sorry for noise, maybe you should try onSliderMoved signal instead.

Diego Casella wrote:
cannot bind to a non-existent property, says plasma ... are you sure this
method exists?

Xuetian Weng wrote:
ah sorry... I initially thought it's Plasma/Slider in cpp but now it
seems it's another implementation in qml.
the cpp version of Plasma/Slider is actually in plasma.graphicswidget but
since we're moving to qt5 in the future graphicswidget implementation is not
a preferable solution...

So you might want to use the solution in my first comment.

Diego Casella wrote:
imho it is too hackish and the applet works good even in this state. Do
we really need to introduce something like that when you change the volume
levels for, say, a total of 5 minutes every day (overstimated)?

Xuetian Weng wrote:
There is some possible data race when last time I looked at the
brightness slider in battery plasmoid. Especially when user drag the slider
which generates more than one onValueChanged (Click on slider is usually ok).

And the slider looks very jumpy in that case.

user drags the slider
onValueChanged caused by user
call setVolume to A to backend
user drags slider further to B
onValueChanged caused by user
call setVolume B to backend

backend notifies the change of A
onValueChanged caused by backend notificatoin
slider change back to A but user mouse is actually already on B.
and now it will send setVolume to A again.
and it will generate another onValueChanged.

Eventually it will have volume at B but if the difference of value A and
B is huge, the visual reflection would be really bad.
And in some case it might generate really quite a lot onValueChanged
(since theoretically it can generate infinite ping-pong loop if all events
arrives just-in-time.).

I didn't test your plasmoid but you can try to drag it aggressively from
on side to the other side to see if this is really a issue.

Believe me, I know exactly the issue you are talking about. I tried various
fixes it since 2010, when I did my first (ugly) proof of concept applet[1] in
C++. Back at that time, i simply disconnected the slider, updated it, and then
restored the connection.
Then, moving to javascript and after that, into qml world, I tried the boolean
solution too, but I realized that: I was adding kind of hackish code to address
an issue which is in fact just a corner case, really. No one (mentally healthy
enough :) would ever drag aggressively the audio slider back and forth and blow
his/her auditive apparatus just because it's funny doing so.
I'm not saying your observation is wrong but, imho, we are adding more code to
cover a really improper usage of the applet which is unlikely to happen.
Let's hear from other devs what they think about it, so we will better decide
whether to deal with that issue or not :)

[1]
http://quickgit.kde.org/?p=scratch%2Fcasella%2Fplasma-applet-kmix.gita=blobh=551c9eab7827c4d1d4a162bb772d9ec30091a1adhb=e5d4ddd21943cbf93166ff8fbd61755ad00d495ef=mixercontrol.cpp#L61

- Diego

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

On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:

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

(Updated Aug. 27, 2013, 8:40 a.m.)

Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and
Igor Poboiko.

Description
---

Re: Review Request 112208: KMix qml applet

On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55

if volume is changed from kmix, not from the applet, this would issue a
duplicate service call to dataengine.

my solution is to add a bool protector to disable this signal if the
data change is not from user.

Xuetian Weng wrote:
sorry for noise, maybe you should try onSliderMoved signal instead.

Diego Casella wrote:
cannot bind to a non-existent property, says plasma ... are you sure this
method exists?

So you might want to use the solution in my first comment.

And the slider looks very jumpy in that case.

user drags the slider
onValueChanged caused by user
call setVolume to A to backend
user drags slider further to B
onValueChanged caused by user
call setVolume B to backend

I didn't test your plasmoid but you can try to drag it aggressively from
on side to the other side to see if this is really a issue.

Diego Casella wrote:
Believe me, I know exactly the issue you are talking about. I tried
various fixes it since 2010, when I did my first (ugly) proof of concept
applet[1] in C++. Back at that time, i simply disconnected the slider,
updated it, and then restored the connection.
Then, moving to javascript and after that, into qml world, I tried the
boolean solution too, but I realized that: I was adding kind of hackish code
to address an issue which is in fact just a corner case, really. No one
(mentally healthy enough :) would ever drag aggressively the audio slider
back and forth and blow his/her auditive apparatus just because it's funny
doing so.
I'm not saying your observation is wrong but, imho, we are adding more
code to cover a really improper usage of the applet which is unlikely to
happen.
Let's hear from other devs what they think about it, so we will better
decide whether to deal with that issue or not :)

[1]
http://quickgit.kde.org/?p=scratch%2Fcasella%2Fplasma-applet-kmix.gita=blobh=551c9eab7827c4d1d4a162bb772d9ec30091a1adhb=e5d4ddd21943cbf93166ff8fbd61755ad00d495ef=mixercontrol.cpp#L61

This can work out differently per machine. I would say that it makes sense to
disable reacting to the changed signal while the user is interacting, so maybe
catch external changes, block them for a short amount when the user has just
changed the value. It's a common case, and can make using the slider
unnaturally jerky, so I think it does warrant some extra complexity in the code
here, hopefully a pretty simple change.

Another solution would be to introduce a sender in the whole volume setting
chain, and make sure to not react to one's own changes after it has done a
round-trip. That would probably be a bit of overkill, though. :)

- Sebastian

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

On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:

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

(Updated Aug. 27, 2013, 8:40 a.m.)

Review request for Plasma, Aaron J. Seigo, Christian Esken,

Re: Review Request 112208: KMix qml applet

On Aug. 22, 2013, 10:46 p.m., Aaron J. Seigo wrote:
File Attachment: Applet Config Options
http://git.reviewboard.kde.org/r/112208/#fcomment91

Do we even really need horizontal/vertical orientation for the sliders
as a configuration option? Other than because we can what is the actual
end user value of that option?

Igor Poboiko wrote:
AFAIK MacOS and Windows use vertical slider orientation, while
GnomeUbuntu use horizontal sliders. Some people may be used to one of the
variants and find another one inconvenient. So why not let them choose? There
is not that many options in that applet anyways.

Marco Martin wrote:
i would prefer not having that as well, plasma appletswere designed to
make easy for the user to change a plasmoid with another if they don't like
it, rther than making the developer maintain many different code paths of
which some combination will always go untested...

one thing without a config dialog that may be tried is adjusting
horizontal or vertical how better suits the size of the config dialog at the
moment...

Sebastian Kügler wrote:
Case in point: The use of QtHorizontal and QtVertical throughout the code
suggest to me that the horizontal case wasn't really tested in the submitted
version. Kinda supports Marco's fear of untested code pathes. ;-)

Diego Casella wrote:
I've tried to keep compatibility with the old KMix interface, which
lets you choose whether you want horizontal or vertical sliders.

@Marco You are completely right, but we don't want to end with two
applets like KMix with horizontal sliders and KMix with vertical sliders
right? We should try to get a reliable procedure to retrieve the size of the
elements in the listview, which is the root of my issues with the applet
resize.

@Sebas Care to explain? Both the cases have been tested.

Sebastian Kügler wrote:
sure, QtVertical and QtHorizontal do not exist, that's a syntax error. I
don't see how that could work, other than by complete accident.

Diego Casella wrote:
Maybe hose enums were kept for backwards compatibilty, because they still
do exists. Right here:

https://projects.kde.org/projects/kde/kde-runtime/repository/revisions/master/entry/plasma/scriptengines/javascript/plasmoid/appletinterface.h#L156

Since my very first draft of the kmix applet is litteraly *years* old,
even though I heavily refactored/modified/dropped the code, the QtVertical
enums is something iI never checked if they were still valid or not, since
they always worked fine :)

Diego Casella wrote:
Out of curiosity: can you now confirm rev1 is working even if it isn't
supposed to, because somehow QtVertical/QtHorizontal are still defined in qml
when they shouldn't?

Do you want to keep the possibility to change the orientation of the
mixer controls?

Diego Casella wrote:
@sebas ping?

They're defined currently, yes. I'm not sure why that is, but that is removed
in Plasma2, and QtHorizontal is Qt.Horizontal and QtVertical is Qt.Vertical. So
it would be better to use Qt.Horizontal and Qt.Vertical throughout, to save
headaches later, and not produce a bad example for others. I've looked into it,
at least now I know how it could work, and how your tests passed. :)

- Sebastian

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

On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:

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

(Updated Aug. 27, 2013, 8:40 a.m.)

Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and
Igor Poboiko.

Description
---

Re: Review Request 112208: KMix qml applet



 On Aug. 22, 2013, 2:34 p.m., Sebastian Kügler wrote:
  File Attachment: Vertical Control
  http://git.reviewboard.kde.org/r/112208/#fcomment92
 
  Clipping
 
 Diego Casella wrote:
 This falls in the known issues - resizing stuff: it is kinda hard to 
 get the sizes right but the applet itself is resizeable so, until we get this 
 right, the user can expand it as much as needed to fix it. That operation 
 needs to be done only once because then plasma keeps track of it, so it would 
 be a single-time hassle.. what do you think?

It should look neat by default. First impression is everything, and the user 
doesn't want to resize all kinds of dialogs, so they fit nicely. This leads to 
a jarring user experience. 
A good way to get to a somewhat adaptable UI is basing the sizing, or the 
margins on font sizes, i.e. theme.defaultFont.mSize, or 
theme.defaultFont.pixelSize, so dpi is taken into account as well. Or just add 
a few pixels of margin.

There's another issue with this, the dialog contents isn't anchored properly to 
the dialog, so resizing doesn't resize the content. This will need to be fixed 
independently, and should hopefully fix the clipping of the last rows, being 
overlaid by the button row.


 On Aug. 22, 2013, 2:34 p.m., Sebastian Kügler wrote:
  plasma/CMakeLists.txt, line 2
  http://git.reviewboard.kde.org/r/112208/diff/1/?file=183950#file183950line2
 
  use the installPackage macro for this and the following line
 
 Diego Casella wrote:
 First it needs to be exported, since it lives inside 
 kde-workspace/plasma/CMakeLists.txt ;)

Ah, ok. In Plasma2, it's indeed in PlasmaMacros, you can keep it that way, and 
we'll change it post-port-to-plasma2. 


- Sebastian


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned

Re: Review Request 112208: KMix qml applet


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


I've installed the applet, and had a look in more detail. It's coming along 
really nicely, and already feels a lot better than the QWidget-based kmix. :)

Some issues I've found:
- Resizing the popup dialog doesn't resize its content, this leads to 
unfriendly resizing on the one hand, and clipping of the list of channel items 
on the other
- The listview is draggable when it doesn't have to be, use something like 
interactive: height  contentsHeight in the ListView to prevent that from 
happening
- The vertical view needs definitely more spacing, if we really want to keep it
- Some spacing would do the listview good, this leads to better grouping of the 
channel items
- The item name isn't properly anchored and should either determine the minimum 
width, or be elided
- The percentage on the left hand side should not be bold
- The slider feels a bit jerky, this is probably the event round-tripping 
Xetuan mentions, see that part of the thread
- The speaker icon feels a bit weird to mute the channel, I think that's 
because it's used with different semantics for the applet as well: open mixer 
(the panel icon). Maybe it could be done with a checkbox, and possibly moved to 
the top-right, so that the layout
- The initial values in the config dialog are not set up correctly. Try 
switching to master channel only, OK, reopen config dialog: it's set to show 
all channels. Haven't checked if this is also the case for the orientation
- Show all mixers - Show all Channels (i.e. does mixer make sense here, or 
is it really a channel, or a Volume Control?
- Mixer slider orientation - Orientation ?

Overall, nice work. This is good stuff. =)

- Sebastian Kügler


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,

Re: Review Request 112208: KMix qml applet



 On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
  http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55
 
  if volume is changed from kmix, not from the applet, this would issue a 
  duplicate service call to dataengine.
  
  my solution is to add a bool protector to disable this signal if the 
  data change is not from user.
 
 Xuetian Weng wrote:
 sorry for noise, maybe you should try onSliderMoved signal instead.

cannot bind to a non-existent property, says plasma ... are you sure this 
method exists?


- Diego


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet



 On Aug. 28, 2013, 9 p.m., Xuetian Weng wrote:
  File Attachment: Control Icon and Label left aligned
  http://git.reviewboard.kde.org/r/112208/#fcomment89
 
  It would be nice to have a line here.
  
  svg: widgets/line and element horizontal-line would work here
 

There's already a line to separate the buttons from the mixer listview. imho 
adding further separators is redundant, we already know that every slider 
refers to the mixer described by the label avobe itself.


- Diego


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet



 On Aug. 29, 2013, 4:05 a.m., Bhushan Shah wrote:
  Can't we have it in branch or hosted somewhere on git?

yep, seems like it would be a good idea indeed ... casella-kmix-qml-applet is 
the branch I made.


- Diego


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

On Aug. 22, 2013, 10:46 p.m., Aaron J. Seigo wrote:
File Attachment: Applet Config Options
http://git.reviewboard.kde.org/r/112208/#fcomment90

Do we even really need horizontal/vertical orientation for the sliders
as a configuration option? Other than because we can what is the actual
end user value of that option?

one thing without a config dialog that may be tried is adjusting
horizontal or vertical how better suits the size of the config dialog at the
moment...

Diego Casella wrote:
I've tried to keep compatibility with the old KMix interface, which
lets you choose whether you want horizontal or vertical sliders.

@Sebas Care to explain? Both the cases have been tested.

Sebastian Kügler wrote:
sure, QtVertical and QtHorizontal do not exist, that's a syntax error. I
don't see how that could work, other than by complete accident.

Diego Casella wrote:
Maybe hose enums were kept for backwards compatibilty, because they still
do exists. Right here:

https://projects.kde.org/projects/kde/kde-runtime/repository/revisions/master/entry/plasma/scriptengines/javascript/plasmoid/appletinterface.h#L156

Diego Casella wrote:
Out of curiosity: can you now confirm rev1 is working even if it isn't
supposed to, because somehow QtVertical/QtHorizontal are still defined in qml
when they shouldn't?

Do you want to keep the possibility to change the orientation of the
mixer controls?

@sebas ping?

- Diego

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

On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:

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

(Updated Aug. 27, 2013, 8:40 a.m.)

Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and
Igor Poboiko.

Description
---

Re: Review Request 112208: KMix qml applet

2013-09-01 Thread Xuetian Weng



 On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
  http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55
 
  if volume is changed from kmix, not from the applet, this would issue a 
  duplicate service call to dataengine.
  
  my solution is to add a bool protector to disable this signal if the 
  data change is not from user.
 
 Xuetian Weng wrote:
 sorry for noise, maybe you should try onSliderMoved signal instead.
 
 Diego Casella wrote:
 cannot bind to a non-existent property, says plasma ... are you sure this 
 method exists?

ah sorry... I initially thought it's Plasma/Slider in cpp but now it seems it's 
another implementation in qml.
the cpp version of Plasma/Slider is actually in plasma.graphicswidget but since 
we're moving to qt5 in the future graphicswidget implementation is not a 
preferable solution...

So you might want to use the solution in my first comment.


- Xuetian


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

2013-08-28 Thread Diego Casella

On Aug. 22, 2013, 10:46 p.m., Aaron J. Seigo wrote:
File Attachment: Applet Config Options
http://git.reviewboard.kde.org/r/112208/#fcomment87

Do we even really need horizontal/vertical orientation for the sliders
as a configuration option? Other than because we can what is the actual
end user value of that option?

one thing without a config dialog that may be tried is adjusting
horizontal or vertical how better suits the size of the config dialog at the
moment...

Diego Casella wrote:
I've tried to keep compatibility with the old KMix interface, which
lets you choose whether you want horizontal or vertical sliders.

@Sebas Care to explain? Both the cases have been tested.

Sebastian Kügler wrote:
sure, QtVertical and QtHorizontal do not exist, that's a syntax error. I
don't see how that could work, other than by complete accident.

Diego Casella wrote:
Maybe hose enums were kept for backwards compatibilty, because they still
do exists. Right here:

https://projects.kde.org/projects/kde/kde-runtime/repository/revisions/master/entry/plasma/scriptengines/javascript/plasmoid/appletinterface.h#L156

Out of curiosity: can you now confirm rev1 is working even if it isn't supposed
to, because somehow QtVertical/QtHorizontal are still defined in qml when they
shouldn't?

Do you want to keep the possibility to change the orientation of the mixer
controls?

- Diego

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

On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:

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

(Updated Aug. 27, 2013, 8:40 a.m.)

Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and
Igor Poboiko.

Description
---

Re: Review Request 112208: KMix qml applet

2013-08-28 Thread Xuetian Weng


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



File Attachment: Control Icon and Label left aligned
http://git.reviewboard.kde.org//r/112208/#fcomment88
It would be nice to have a line here.

svg: widgets/line and element horizontal-line would work here


- Xuetian Weng


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

2013-08-28 Thread Xuetian Weng


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



plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml
http://git.reviewboard.kde.org/r/112208/#comment28679

if volume is changed from kmix, not from the applet, this would issue a 
duplicate service call to dataengine.

my solution is to add a bool protector to disable this signal if the data 
change is not from user.


- Xuetian Weng


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

2013-08-28 Thread Xuetian Weng



 On Aug. 28, 2013, 9:10 p.m., Xuetian Weng wrote:
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml, line 55
  http://git.reviewboard.kde.org/r/112208/diff/1/?file=183953#file183953line55
 
  if volume is changed from kmix, not from the applet, this would issue a 
  duplicate service call to dataengine.
  
  my solution is to add a bool protector to disable this signal if the 
  data change is not from user.

sorry for noise, maybe you should try onSliderMoved signal instead.


- Xuetian


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

2013-08-28 Thread Bhushan Shah


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


Can't we have it in branch or hosted somewhere on git?

- Bhushan Shah


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet



 On Aug. 26, 2013, 3:38 p.m., Igor Poboiko wrote:
  File Attachment: Vertical Control
  http://git.reviewboard.kde.org/r/112208/#fcomment83
 
  Do we need it? 
  
  (there is volume level percentage under the tooltip)
  
  It isn't consistent with horizontal view where there is no such label. 
  Also it duplicates the information from slider. And also it reacts pretty 
  slow to the changes when user moves the slider (due to slowness of the 
  chain plasma-dbus-kmix-backend-kmix-dbus-plasma), which is pretty 
  frustrating.
  
  A label (or a tooltip maybe?) with device name would be nice, because 
  it is pretty hard to decide what volume am I changing when there are 
  multiple cards (or just input and output sliders like in PulseAudio).

tooltpis are gone in rev2, see sebas comment ;)
The reason I didn't used a label with this layout is that it would take sooo 
much space but sure, this is an issue that must be addressed because the user 
must know which control he/she's going to modify ... I'll try to play with some 
wrapMode Label properties and see if I can get something usable and good 
looking.


- Diego


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


On Aug. 24, 2013, 3:11 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 24, 2013, 3:11 p.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/config/main.xml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/ButtonBar.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/MixersList.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/config.ui PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/kmixapplet.qml PRE-CREATION 
   plasma/kmix-applet-qml/metadata.desktop PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet



 On Aug. 22, 2013, 2:34 p.m., Sebastian Kügler wrote:
  File Attachment: Menu Actions
  http://git.reviewboard.kde.org/r/112208/#fcomment84
 
  Maybe we could align this in the same way as the batter applet does?
 
 Diego Casella wrote:
 The menu comes from a right-click from within the widget -not the applet 
 icon in the panel-, but i forgot to check the include mouse pointer option 
 when I took that screenshot :)
 
 Martin Klapetek wrote:
 (the comment was about the plasmoid stuff alignment, not the menu ;)
 
 I'd agree with aligning it the same way as battery plasmoid - put the app 
 icon on the left, make it span over two rows and discard the speaker icon. 
 Also the text should be aligned to left then instead of centered, looks a bit 
 chaotic imho.
 
 That way we'll have consistent look in two important parts of the 
 workspace, which I believe is very important.

I see, my bad.
Pretty easy fix, will submit the update in a min ;)


- Diego


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


On Aug. 24, 2013, 3:11 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 24, 2013, 3:11 p.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/config/main.xml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/ButtonBar.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/MixersList.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/config.ui PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/kmixapplet.qml PRE-CREATION 
   plasma/kmix-applet-qml/metadata.desktop PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet


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

(Updated Aug. 27, 2013, 8:39 a.m.)


Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
Igor Poboiko.


Changes
---

Layout icon and control label on the left as david suggested


Description
---

KMix qml applet.
As you can see from the screenshot, the applet is pretty much functional: you 
can display all the controls available, change its orientation, and decide to 
whether show all of them or just the Master Control, and refresh its status 
when new controls are added/removed/updated (such as Amarok current playing 
track). See screenshots below :)
Differences from the old kmix tray:
* no media player controls ( I never investigated how to get them, but honestly 
opening the audio applet to change/skip/pause audio track makes little sense to 
me ... if anyone wants this feature back, don't be shy and step in);
* the button used to select which Mixers are visible has been changed to open 
Phonon kcm page: since visible mixers are already configurable from KMix app, 
having a button to show KMix *and* a button to modify Mixers visibilty made 
little sense here too, so I preferred to give more visibility to Phonon kcm;

Known issues:
* there is still no way to get notified of mouse wheel events over the 
popupIcon, so it is not possible to scroll over to increase/decrease the master 
control volume;
* no scroll events over the sliders too;
* if you want to use the applet you most likely will disable KMix tray icon 
but, if you do so, KMix will show its GUI at every login and you have to close 
it manually. This requires KMix to be patched. Furthermore, if you click KMix 
Setup button, KMix window will not restored anymore: this needs to be pathed 
as well.
* resize doesn't work properly.


Diffs (updated)
-

  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/112208/diff/


Testing
---

Tested against master and works fine.


File Attachments


Default look
  http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
Menu Actions
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
Applet Config Options
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
Vertical Control
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
ToolButton label and Config page after updates
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png


Thanks,

Diego Casella

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

Re: Review Request 112208: KMix qml applet


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

(Updated Aug. 27, 2013, 8:40 a.m.)


Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
Igor Poboiko.


Description
---

KMix qml applet.
As you can see from the screenshot, the applet is pretty much functional: you 
can display all the controls available, change its orientation, and decide to 
whether show all of them or just the Master Control, and refresh its status 
when new controls are added/removed/updated (such as Amarok current playing 
track). See screenshots below :)
Differences from the old kmix tray:
* no media player controls ( I never investigated how to get them, but honestly 
opening the audio applet to change/skip/pause audio track makes little sense to 
me ... if anyone wants this feature back, don't be shy and step in);
* the button used to select which Mixers are visible has been changed to open 
Phonon kcm page: since visible mixers are already configurable from KMix app, 
having a button to show KMix *and* a button to modify Mixers visibilty made 
little sense here too, so I preferred to give more visibility to Phonon kcm;

Known issues:
* there is still no way to get notified of mouse wheel events over the 
popupIcon, so it is not possible to scroll over to increase/decrease the master 
control volume;
* no scroll events over the sliders too;
* if you want to use the applet you most likely will disable KMix tray icon 
but, if you do so, KMix will show its GUI at every login and you have to close 
it manually. This requires KMix to be patched. Furthermore, if you click KMix 
Setup button, KMix window will not restored anymore: this needs to be pathed 
as well.
* resize doesn't work properly.


Diffs
-

  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/112208/diff/


Testing
---

Tested against master and works fine.


File Attachments (updated)


Default look
  http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
Menu Actions
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
Applet Config Options
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
Vertical Control
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
ToolButton label and Config page after updates
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
Control Icon and Label left aligned
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png


Thanks,

Diego Casella

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

Re: Review Request 112208: KMix qml applet

2013-08-27 Thread Martin Klapetek


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



File Attachment: Control Icon and Label left aligned
http://git.reviewboard.kde.org//r/112208/#fcomment85
I wonder if we still need the speaker icons. The way I see it they 
represent two things - one is how loud is this sound (higher the volume is, 
more arcs the icon has). This is also represented by the slider and then by the 
percentage string too, so the volume information is there 3 times. The second 
thing it does is saying this is a volume control. This on the other hand is 
quite important I think, although if you popup kmix applet (which has the 
speaker icon again), I guess one expects the sliders to be volume adjusting 
sliders. Dunno, I think the plasmoid would be clearer without the speaker icons.

Either way, looks good when left aligned, good job!

- Martin Klapetek


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet



 On Aug. 27, 2013, 10:57 a.m., Martin Klapetek wrote:
  File Attachment: Control Icon and Label left aligned
  http://git.reviewboard.kde.org/r/112208/#fcomment86
 
  I wonder if we still need the speaker icons. The way I see it they 
  represent two things - one is how loud is this sound (higher the volume 
  is, more arcs the icon has). This is also represented by the slider and 
  then by the percentage string too, so the volume information is there 3 
  times. The second thing it does is saying this is a volume control. This 
  on the other hand is quite important I think, although if you popup kmix 
  applet (which has the speaker icon again), I guess one expects the sliders 
  to be volume adjusting sliders. Dunno, I think the plasmoid would be 
  clearer without the speaker icons.
  
  Either way, looks good when left aligned, good job!

That is a plain Plasma ToolButton, not a simple icon, which does the same as 
the button you can find in the old KMix interface does. If you click it, the 
corresponding channel will be muted. If it's already muted, it will be un-muted 
:)
imho the label gives a more concise/precise info about the actual volume level, 
which the slider cannot offer because of its nature.


- Diego


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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

Re: Review Request 112208: KMix qml applet

2013-08-26 Thread Martin Klapetek



 On Aug. 22, 2013, 2:34 p.m., Sebastian Kügler wrote:
  File Attachment: Menu Actions
  http://git.reviewboard.kde.org/r/112208/#fcomment80
 
  KMix and Phonon are jargon and have to go.
  
  Proposal:
  
  Mixer Setup
  Audio Setup
  
  Seems more in line with kmix

Plus it should use elipsis on the end, eg. Mixer Setup... and Audio 
Setup


 On Aug. 22, 2013, 2:34 p.m., Sebastian Kügler wrote:
  File Attachment: Menu Actions
  http://git.reviewboard.kde.org/r/112208/#fcomment81
 
  Maybe we could align this in the same way as the batter applet does?
 
 Diego Casella wrote:
 The menu comes from a right-click from within the widget -not the applet 
 icon in the panel-, but i forgot to check the include mouse pointer option 
 when I took that screenshot :)

(the comment was about the plasmoid stuff alignment, not the menu ;)

I'd agree with aligning it the same way as battery plasmoid - put the app icon 
on the left, make it span over two rows and discard the speaker icon. Also the 
text should be aligned to left then instead of centered, looks a bit chaotic 
imho.

That way we'll have consistent look in two important parts of the workspace, 
which I believe is very important.


- Martin


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


On Aug. 24, 2013, 3:11 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 24, 2013, 3:11 p.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/config/main.xml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/ButtonBar.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/MixersList.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/config.ui PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/kmixapplet.qml PRE-CREATION 
   plasma/kmix-applet-qml/metadata.desktop PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 
 
 Thanks,
 
 Diego Casella
 


___
Plasma-devel

Re: Review Request 112208: KMix qml applet


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

(Updated Aug. 24, 2013, 3:07 p.m.)


Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
Igor Poboiko.


Changes
---

Update diff according to (almost all) the observations made so far.
Moved portions of code to be plasma-qml-style compliant :)


Description
---

KMix qml applet.
As you can see from the screenshot, the applet is pretty much functional: you 
can display all the controls available, change its orientation, and decide to 
whether show all of them or just the Master Control, and refresh its status 
when new controls are added/removed/updated (such as Amarok current playing 
track). See screenshots below :)
Differences from the old kmix tray:
* no media player controls ( I never investigated how to get them, but honestly 
opening the audio applet to change/skip/pause audio track makes little sense to 
me ... if anyone wants this feature back, don't be shy and step in);
* the button used to select which Mixers are visible has been changed to open 
Phonon kcm page: since visible mixers are already configurable from KMix app, 
having a button to show KMix *and* a button to modify Mixers visibilty made 
little sense here too, so I preferred to give more visibility to Phonon kcm;

Known issues:
* there is still no way to get notified of mouse wheel events over the 
popupIcon, so it is not possible to scroll over to increase/decrease the master 
control volume;
* no scroll events over the sliders too;
* if you want to use the applet you most likely will disable KMix tray icon 
but, if you do so, KMix will show its GUI at every login and you have to close 
it manually. This requires KMix to be patched. Furthermore, if you click KMix 
Setup button, KMix window will not restored anymore: this needs to be pathed 
as well.
* resize doesn't work properly.


Diffs (updated)
-

  plasma/kmix-applet-qml/contents/config/main.xml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/ButtonBar.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/MixersList.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/config.ui PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/kmixapplet.qml PRE-CREATION 
  plasma/kmix-applet-qml/metadata.desktop PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/112208/diff/


Testing
---

Tested against master and works fine.


File Attachments


Default look
  http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
Menu Actions
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
Applet Config Options
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
Vertical Control
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png


Thanks,

Diego Casella

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

Re: Review Request 112208: KMix qml applet


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

(Updated Aug. 24, 2013, 3:11 p.m.)


Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
Igor Poboiko.


Changes
---

sorry, forgot to add a screenshot for the updated version.


Description
---

KMix qml applet.
As you can see from the screenshot, the applet is pretty much functional: you 
can display all the controls available, change its orientation, and decide to 
whether show all of them or just the Master Control, and refresh its status 
when new controls are added/removed/updated (such as Amarok current playing 
track). See screenshots below :)
Differences from the old kmix tray:
* no media player controls ( I never investigated how to get them, but honestly 
opening the audio applet to change/skip/pause audio track makes little sense to 
me ... if anyone wants this feature back, don't be shy and step in);
* the button used to select which Mixers are visible has been changed to open 
Phonon kcm page: since visible mixers are already configurable from KMix app, 
having a button to show KMix *and* a button to modify Mixers visibilty made 
little sense here too, so I preferred to give more visibility to Phonon kcm;

Known issues:
* there is still no way to get notified of mouse wheel events over the 
popupIcon, so it is not possible to scroll over to increase/decrease the master 
control volume;
* no scroll events over the sliders too;
* if you want to use the applet you most likely will disable KMix tray icon 
but, if you do so, KMix will show its GUI at every login and you have to close 
it manually. This requires KMix to be patched. Furthermore, if you click KMix 
Setup button, KMix window will not restored anymore: this needs to be pathed 
as well.
* resize doesn't work properly.


Diffs
-

  plasma/kmix-applet-qml/contents/config/main.xml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/ButtonBar.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/MixersList.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/config.ui PRE-CREATION 
  plasma/kmix-applet-qml/contents/ui/kmixapplet.qml PRE-CREATION 
  plasma/kmix-applet-qml/metadata.desktop PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/112208/diff/


Testing
---

Tested against master and works fine.


File Attachments (updated)


Default look
  http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
Menu Actions
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
Applet Config Options
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
Vertical Control
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
ToolButton label and Config page after updates
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png


Thanks,

Diego Casella

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

Re: Review Request 112208: KMix qml applet



 On Aug. 22, 2013, 10:46 p.m., Aaron J. Seigo wrote:
  File Attachment: Applet Config Options
  http://git.reviewboard.kde.org/r/112208/#fcomment77
 
  Do we even really need horizontal/vertical orientation for the sliders 
  as a configuration option? Other than because we can what is the actual 
  end user value of that option?
 
 Igor Poboiko wrote:
 AFAIK MacOS and Windows use vertical slider orientation, while 
 GnomeUbuntu use horizontal sliders. Some people may be used to one of the 
 variants and find another one inconvenient. So why not let them choose? There 
 is not that many options in that applet anyways.
 
 Marco Martin wrote:
 i would prefer not having that as well, plasma appletswere designed to 
 make easy for the user to change a plasmoid with another if they don't like 
 it, rther than making the developer maintain many different code paths of 
 which some combination will always go untested...
 
 one thing without a config dialog that may be tried is adjusting 
 horizontal or vertical how better suits the size of the config dialog at the 
 moment...
 
 Sebastian Kügler wrote:
 Case in point: The use of QtHorizontal and QtVertical throughout the code 
 suggest to me that the horizontal case wasn't really tested in the submitted 
 version. Kinda supports Marco's fear of untested code pathes. ;-)

I've tried to keep compatibility with the old KMix interface, which lets you 
choose whether you want horizontal or vertical sliders.

@Marco You are completely right, but we don't want to end with two applets like 
KMix with horizontal sliders and KMix with vertical sliders right? We 
should try to get a reliable procedure to retrieve the size of the elements in 
the listview, which is the root of my issues with the applet resize.

@Sebas Care to explain? Both the cases have been tested.


- Diego


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


On Aug. 24, 2013, 3:11 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 24, 2013, 3:11 p.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/config/main.xml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/ButtonBar.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/MixersList.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/config.ui PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/kmixapplet.qml PRE-CREATION 
   plasma/kmix-applet-qml/metadata.desktop PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look

Re: Review Request 112208: KMix qml applet