<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/127476/">https://git.reviewboard.kde.org/r/127476/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 24th, 2016, 6:25 a.m. UTC, <b>David Rosca</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please see https://git.reviewboard.kde.org/r/125088/. I myself am against blocking it, but there is a valid reason for it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also, I think you can just remove the MouseArea (as Slider allows to change value with mouse wheel by default) instead of reimplementing the logic.</p></pre>
</blockquote>
<p>On March 24th, 2016, 7:35 a.m. UTC, <b>Anthony Fieroni</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not removing MouseArea to not fooling the updateTimer, if it can happen. Actual volume is changed in applet and kcm, current code of src/kcm/package/contents/ui/VolumeSlider.qml is without MouseArea, i can remove it.
Same code on 2 side => horrible nightmare</p></pre>
</blockquote>
<p>On March 24th, 2016, 8:35 a.m. UTC, <b>David Rosca</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What's the problem with updateTimer? The code should be identical with KCM, except there is a MouseArea that blocks wheel events in applet. So if it doesn't work, it should be fixed there too.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Regarding the wheel blocking, I think we should instead of blocking wheel on sliders block wheel to scroll view when mouse is on/above/below slider - enable scrolling only when mouse is on the sides (on the volume icon on the left and percent label on the right). That way we have wheel changing volume on sliders, don't have accidental volume change when scrolling but still are able to scroll the view on the sides.</p></pre>
</blockquote>
<p>On March 24th, 2016, 8:46 a.m. UTC, <b>Anthony Fieroni</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;"><ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">updateTimer can run and wheel may change position who reflect on onValueChanged and logic can go down, easly. About me this patch is needed for kcm too, i may wrong. :)</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">I test, mouse wheel is only on wiew item.</li>
</ol></pre>
</blockquote>
<p>On March 24th, 2016, 9:04 a.m. UTC, <b>David Rosca</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can you please elaborate? It shouldn't matter if updateTimer is running, as wheel event will result in valueChanged and that will restart the timer. I'm not saying there is not an issue, right now I managed to end in infinite loop when messing with slider (can't reproduce it again...), but I'd like to understand it.</p></pre>
</blockquote>
<p>On March 24th, 2016, 11:47 a.m. UTC, <b>Anthony Fieroni</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll remove MouseArea and rewrite timer logic, it seems wrong. I think we must set volume and wait for feedback (pulse can ignore it)</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Currently it changes the slider value instantly and then sync the value with pulse after 200msecs, so it does handle the case when pulse ignores the change.</p></pre>
<br />
<p>- David</p>
<br />
<p>On March 23rd, 2016, 5:43 p.m. UTC, Anthony Fieroni 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 Plasma and Harald Sitter.</div>
<div>By Anthony Fieroni.</div>
<p style="color: grey;"><i>Updated March 23, 2016, 5:43 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-pa
</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;">I don't know why it's disabled, it's pretty handy</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>applet/contents/ui/ListItemBase.qml <span style="color: grey">(2e31eeb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127476/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>