> On Aug. 22, 2013, 10:46 p.m., Aaron J. Seigo wrote:
> > File Attachment: Applet Config Options
> > <http://git.reviewboard.kde.org/r/112208/#fcomment79>
> >
> >     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 
> Gnome&Ubuntu 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.

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


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

Reply via email to