<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/124986/">https://git.reviewboard.kde.org/r/124986/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 30th, 2015, 2:10 p.m. UTC, <b>Thomas Pfeiffer</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;">Usability-wise it's fine.
Visually, the clear buttons for the shortcuts should be right-aligned</p></pre>
</blockquote>
<p>On August 30th, 2015, 2:22 p.m. UTC, <b>Kai Uwe Broulik</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;">They are? Or what are you referring to?</p></pre>
</blockquote>
<p>On August 30th, 2015, 6:54 p.m. UTC, <b>David Edmundson</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;">@Thomas, FYI if you click on the screenshot you can draw a rectangle over bits you want to comment on</p></pre>
</blockquote>
<p>On August 30th, 2015, 7:46 p.m. UTC, <b>Hugo Pereira Da Costa</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 think (and am possibly wrong) that Thomas is talking about the fact that the clear buttons of the first two lines in the list from the screenshot are not aligned (neither left nor right) with the ones on all lines below. he would make them right aligned across all lines in the list.
Also (and it is likely unrelated to this patch), what about the horizontal alignment of the list with respect to the checkbox just above ?</p></pre>
</blockquote>
<p>On August 30th, 2015, 7:48 p.m. UTC, <b>Kai Uwe Broulik</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;">That's just QQ Layouts being weird, it pulls apart the two buttons of the KeySequenceItem. I could only make the clear button stick to the other one.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What's with the CheckBox? The old list was indented to show the checkbox turns them all off, that doesn't neccessarily hold with the new TableView, though.</p></pre>
</blockquote>
<p>On August 31st, 2015, 12:48 a.m. UTC, <b>Thomas Pfeiffer</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's just QQ Layouts being weird, it pulls apart the two buttons of the KeySequenceItem. I could only make the clear button stick to the other one.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does that mean it cannot be fixed?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What's with the CheckBox? The old list was indented to show the checkbox turns them all off, that doesn't neccessarily hold with the new TableView, though.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">https://techbase.kde.org/Projects/Usability/HIG/Alignment says "When options are subordinate to a check box or radio button (e.g. Audio level can only be set if the Activate Audio option is selected), this relation should be visualized by indenting the sub-options."</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;">I got feedback from our visual designers, and they decided that right-aligning both the clear buttons and the actual shortcuts (each only among themselves, of course) would be the way to go. (To be sure: We mean what you showed me in the screenshot, Kai).</p></pre>
<br />
<p>- Thomas</p>
<br />
<p>On August 29th, 2015, 9:25 p.m. UTC, Kai Uwe Broulik 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 KDE Usability.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Aug. 29, 2015, 9:25 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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;">In Plasma 4.x you could configure applet shortcuts from System Tray settings rather fiddling around with each individual applet. This restores this.</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">It uses a TableView like the 4.x did (TableView is a good candidate for pushing ComboBox off its throne of horribleness), including the item's icon</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Shortcuts in System Tray are now handled through QKeySequence rather than QString</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">When checking "Always show all entries" the comboboxes become disabled (not the entire Table so you could still change shortcuts)</li>
</ul></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;">I can set and unset applet shortcuts (did we have that for DBus tasks? There were stubs for that but I can't remember), as well as hide/show/auto entries.</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>applets/systemtray/package/contents/ui/ConfigEntries.qml <span style="color: grey">(e848ae3)</span></li>
<li>applets/systemtray/plugin/protocols/dbussystemtray/dbussystemtraytask.h <span style="color: grey">(f499a57)</span></li>
<li>applets/systemtray/plugin/protocols/dbussystemtray/dbussystemtraytask.cpp <span style="color: grey">(6486f18)</span></li>
<li>applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.h <span style="color: grey">(74d1e1f)</span></li>
<li>applets/systemtray/plugin/protocols/plasmoid/plasmoidtask.cpp <span style="color: grey">(1528c7a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124986/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/08/29/795452f7-f817-49d5-a2bb-ba43234f4f91__systemtrayshortcuts.png">UI in action</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>