<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123121/">https://git.reviewboard.kde.org/r/123121/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would like a bit more spacing, so I added it. But apart from that the patch is fine. Two comments though for future developers:
1) The patch operates on the value of one individual channel slider, and thus is representing only the value of that channel. This is technically not 100% sound for a corner case: The average of all channels should be displayed, as given by the "underlying" Volume class. On the other hand, in "joined slider mode", the slider should already represent the average of all channels. And as soon as you move the slider, all channels are changed. So it is likely really exact enough.
2) Future directions: It would be better to do the percentage calculation via the Volume class, as it handles corner cases like muting. But due to "auto-unmuting" the value is factually also correct. As there is no pointer/reference to the "underlying" Volume object in the MDWSlider, I would say: Just keep it like this.</p></pre>
<br />
<p>- Christian Esken</p>
<br />
<p>On April 25th, 2015, 4:26 p.m. UTC, Ronnie Thomas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Multimedia and Christian Esken.</div>
<div>By Ronnie Thomas.</div>
<p style="color: grey;"><i>Updated April 25, 2015, 4:26 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=320949">320949</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kmix
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch adds a tooltip to the VolumeSlider, which shows the current value of the slider in percentage. The tooltip is shown only when the slider handle is selected, and it changes its value dynamically when the slider is moved from its current position.
Possible implementation of bug 320949.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested for both horizontal and vertical orientation of the slider.</p></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>gui/volumeslider.h <span style="color: grey">(616c9a0)</span></li>
<li>gui/volumeslider.cpp <span style="color: grey">(1a4166e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123121/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>