<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/6928/">http://svn.reviewboard.kde.org/r/6928/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 26th, 2013, 10:30 p.m. UTC, <b>Christian Esken</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have not followed this request, but it looks like I have not receieved any followups on my last comment. I would like to know what to do with this review request. Diego, Igor? Diego, are you still following up on this. Care to contact Igor?</pre>
</blockquote>
<p>On July 27th, 2013, 2:23 p.m. UTC, <b>Diego Casella</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi Christian, after hearing that someone else was working on it, I left the development of that plasmoid; I didnt' want to step over Igor's work.
Anyway, by looking at the current kmix status, it's still missing the conversion to a full kded service: otherwise there will be two entries in the tray: the "old" kmix, and its plasmoid counterpart. We also need to extend its dbus interface and add a method to pop-up the full kmix gui: in that way, the plasmoid can show the user interface, allowing the end-user to configure kmix properly.
As soon as those changes will be peformed (if you agree with it of course, and if you want to implement the new kmix as a qml plasmoid), I'd be happy to continue what I did so far :)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Commenting on:
"if you want to implement the new kmix as a qml plasmoid"
Well, I have no knowledge in QML and no desire to learn it. If you still want to do so - focussing on the tray popup/plasmoid - then it should be much more functional than your prototype - especially: supporting multiple controls, adding Media player control like in todays KMix.
The KMix Mainwindow will surely always stay a normal application, everything else is a complete new application.</pre>
<br />
<p>- Christian</p>
<br />
<p>On March 28th, 2012, 6:49 p.m. UTC, Diego Casella wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma, Aaron Seigo, Christian Esken, and Marco Martin.</div>
<div>By Diego Casella.</div>
<p style="color: grey;"><i>Updated March 28, 2012, 6:49 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">First attempt of making a declarative kmix applet for plasma.
What the apple does right now:
* modifies the volume level and the mute/unmute status of the master channel;
* reacts to changes of the volume level/status (i.e. made with multimedia keys);
* disables the slider if the channel gets muted, and enables it back as soon as the channel gets unmuted;
* collapses gracefully in a popup icon when placed inside the panel.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested against r1287510. For basic audio management it works great imho.
However, there is a lot of room for improvements, but this is gonna need some extra work outside the kmix applet scope:
* first of all, the applet need kmix executable to run in order to perform the dbus calls. You can of course disable KMix tray icon feature but, at every login, KMix mainwindow will be shown and the user must closeby hand. This is a kind of ugly behavior that should be avoided;
* it will be great to great to add an action to allow the user to select the master channel (by reusing KMix "Select Master Channel" widget), but this will require tweaking KMix dbus interface;
* as you noticed in the screenshots, the applet in the panel and in the desktop have different size even if it __is__ actually the same: something is going wrong when plasma shows the PopupApplet. This behavior was even worse when I started implementing a "flip" action to change the layout from horizontal to vertical and vice-versa, and for this reason I gave up and simply stick with the vertical layout.
Could this applet be shipped in the current status, or should we wait for all the aforementioned improvements?
Comments/ideas/suggestions?
Cheers :)</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt <span style="color: grey">(1287513)</span></li>
<li>trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/VerticalControl.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/kmixapplet.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/metadata.desktop <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6928/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://svn.reviewboard.kde.org/r/6928/s/627/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2012/03/28/kmixapplet_400x100.png" style="border: 1px black solid;" alt="Applet look in panel and desktop" /></a>
<a href="http://svn.reviewboard.kde.org/r/6928/s/628/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2012/03/28/kmixapplet_muted_400x100.png" style="border: 1px black solid;" alt="Applet look in panel and desktop - audio muted" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>