> 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? > > 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. > > 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 :)
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 > ------- > > 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
