<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/129221/">https://git.reviewboard.kde.org/r/129221/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On říjen 20th, 2016, 8:39 dop. UTC, <b>Heiko Tietze</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;">Spacing has a meaning. When you, for instance in image 3, have more space between an upper and a lower group I wonder what the reason is. And image 2 has only 1 or 2px instead of ~6px in the other. I suggest you define one distance and use this values for all controls (no need to stick to the exact values at the HIG https://community.kde.org/KDE_Visual_Design_Group/HIG/Placement since it was a proposal back than and still is). And if there a good reason to have more space you should have a second value.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You correctly right align captions but violate this in the last two images. The alignment HIG is a little bit more elaborated https://community.kde.org/KDE_Visual_Design_Group/HIG/Alignment.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"Hidden network" is still missing text after the checkbox. It's needed at least for a11y. https://community.kde.org/KDE_Visual_Design_Group/HIG/CheckBox ("</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Password inputs still use the inline storage option. For consistency, you better move it out of the control like discussed in https://git.reviewboard.kde.org/r/129212/ (personally I never had a problem with the inline solution).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Noticed the colon after captions: While it's perhaps today's standard I think the UX specialists should consider a guideline that colons shouldn't be used for labels.</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;">To explain the pictures better, there is always a picture how it was done before and how it looks with this patch. So image 1 is old IPv4 configuration and image 2 is with this patch. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Password still uses the inline storage option, because I have that patch somewhere else and given it's not pushed it would complicate me doing diffs.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We can remove colons later if we agree on that.</p></pre>
<br />
<p>- Jan</p>
<br />
<p>On říjen 20th, 2016, 8:13 dop. UTC, Jan Grulich 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 Network Management, KDE Usability and Lamarque Souza.</div>
<div>By Jan Grulich.</div>
<p style="color: grey;"><i>Updated Říj. 20, 2016, 8:13 dop.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-nm
</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;"><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;">vertical spacing between all widgets set to 6px (previously only few of them had this spacing)</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">checkbox alignment according to HIG</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">correct label alignment for some VPN widgets</li>
</ul></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>libs/editor/settings/ipv4widget.cpp <span style="color: grey">(2e92061)</span></li>
<li>libs/editor/settings/ipv6widget.cpp <span style="color: grey">(c5fa3b8)</span></li>
<li>libs/editor/settings/ui/802-1x.ui <span style="color: grey">(54ae4fc)</span></li>
<li>libs/editor/settings/ui/bond.ui <span style="color: grey">(1c7d1c3)</span></li>
<li>libs/editor/settings/ui/bridge.ui <span style="color: grey">(61a1db6)</span></li>
<li>libs/editor/settings/ui/bt.ui <span style="color: grey">(295cd9e)</span></li>
<li>libs/editor/settings/ui/cdma.ui <span style="color: grey">(bace7ce)</span></li>
<li>libs/editor/settings/ui/infiniband.ui <span style="color: grey">(a3ab450)</span></li>
<li>libs/editor/settings/ui/ipv4.ui <span style="color: grey">(0aae15e)</span></li>
<li>libs/editor/settings/ui/ipv6.ui <span style="color: grey">(58d0c89)</span></li>
<li>libs/editor/settings/ui/ppp.ui <span style="color: grey">(f53f38b)</span></li>
<li>libs/editor/settings/ui/pppoe.ui <span style="color: grey">(bc6ee35)</span></li>
<li>libs/editor/settings/ui/vlan.ui <span style="color: grey">(1ea5bae)</span></li>
<li>libs/editor/settings/ui/wificonnectionwidget.ui <span style="color: grey">(283cff4)</span></li>
<li>libs/editor/settings/ui/wifisecurity.ui <span style="color: grey">(27610d0)</span></li>
<li>libs/editor/settings/ui/wiredconnectionwidget.ui <span style="color: grey">(c07ae7f)</span></li>
<li>vpn/iodine/iodine.ui <span style="color: grey">(ac92c17)</span></li>
<li>vpn/l2tp/l2tp.ui <span style="color: grey">(3dedc44)</span></li>
<li>vpn/l2tp/l2tpadvanced.ui <span style="color: grey">(80e333f)</span></li>
<li>vpn/l2tp/l2tpauth.ui <span style="color: grey">(a5de7fe)</span></li>
<li>vpn/l2tp/l2tpppp.ui <span style="color: grey">(64ce2ac)</span></li>
<li>vpn/openconnect/openconnectprop.ui <span style="color: grey">(ed7dfde)</span></li>
<li>vpn/openswan/openswan.ui <span style="color: grey">(b5e107d)</span></li>
<li>vpn/openswan/openswanauth.ui <span style="color: grey">(aa7f626)</span></li>
<li>vpn/openvpn/openvpnadvanced.ui <span style="color: grey">(24f7564)</span></li>
<li>vpn/pptp/pptpadvanced.ui <span style="color: grey">(dd51bc7)</span></li>
<li>vpn/pptp/pptpprop.ui <span style="color: grey">(33c1e86)</span></li>
<li>vpn/ssh/sshwidget.ui <span style="color: grey">(95b4b6d)</span></li>
<li>vpn/sstp/sstpadvanced.ui <span style="color: grey">(d198620)</span></li>
<li>vpn/sstp/sstpwidget.ui <span style="color: grey">(c4a0b42)</span></li>
<li>vpn/vpnc/vpnc.ui <span style="color: grey">(ab0ba36)</span></li>
<li>vpn/vpnc/vpncadvanced.ui <span style="color: grey">(9c27f85)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129221/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>