<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/125723/">https://git.reviewboard.kde.org/r/125723/</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 21st, 2015, 7:41 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;">Looks very nice. But the question is whether or not users understand the status quickly and if the dropdown menu is clear enough (the eye is a toggle button). The alternative solution is just a radio button group below the input, which is furthermore easily accessible by disabled people.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd place the "ask everytime" option as the first item since it is the default.</p></pre>
</blockquote>
<p>On říjen 21st, 2015, 8:06 dop. UTC, <b>Jan Grulich</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;">Having a radio button group below each password field would make it heavy I would say. I'm trying to remove as much visible components I can to make it more simple.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The "ask everytime" option is not the default option, the default option should be to store secrets for the current user which should store secrets to KWallet.</p></pre>
</blockquote>
<p>On říjen 21st, 2015, 9:58 dop. UTC, <b>Lamarque Souza</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 agree with Jan that radio button group would pollute the ui. Talking about the default option, wouldn't be better make "Always Ask" the default for VPN connections? I know that is not consistent with the current default, but VPN is kind of a special case since usually whoever is using it is more concerned about security than convenience. By the way, good job at improving Plasma NM.</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 agree, using "always ask" as default for VPN connections is something we should consider. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And thanks, plasma-nm is my passion so I don't have to force myself to make it better :).</p></pre>
<br />
<p>- Jan</p>
<br />
<p>On říjen 21st, 2015, 6:12 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. 21, 2015, 6:12 dop.</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=340707">340707</a>
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I took again inspiration from nm-connection-editor and extended our editor to allow to users select password storage. Now each password field has an option to either store password to all users (store it to NM) or to current user (store it to KWallet) or do not store it at all (user will be prompted) or tell NM that the password is not required at all (available only for some connection types). Also during transformation of strongswan vpn plugin I found out that strongswan shouldn't support password storing at all (according to NM plugin) so I removed it accordingly.</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;">this patch also tries to fix couple of coding style issues as usual</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Screenshot:
<img alt="Screenshot of new options" src="https://jgrulich.fedorapeople.org/nm-password-options.png" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /></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>libs/editor/connectiondetaileditor.cpp <span style="color: grey">(aa09d1f)</span></li>
<li>libs/editor/settings/bondwidget.h <span style="color: grey">(2c64c31)</span></li>
<li>libs/editor/settings/bondwidget.cpp <span style="color: grey">(dbd2cf2)</span></li>
<li>libs/editor/settings/bridgewidget.h <span style="color: grey">(a322083)</span></li>
<li>libs/editor/settings/bridgewidget.cpp <span style="color: grey">(3dc1889)</span></li>
<li>libs/editor/settings/btwidget.h <span style="color: grey">(84a8f71)</span></li>
<li>libs/editor/settings/btwidget.cpp <span style="color: grey">(e2e9972)</span></li>
<li>libs/editor/settings/cdmawidget.h <span style="color: grey">(bbffe0d)</span></li>
<li>libs/editor/settings/cdmawidget.cpp <span style="color: grey">(f361d9e)</span></li>
<li>libs/editor/settings/gsmwidget.h <span style="color: grey">(69d4cfd)</span></li>
<li>libs/editor/settings/gsmwidget.cpp <span style="color: grey">(941f411)</span></li>
<li>libs/editor/settings/infinibandwidget.h <span style="color: grey">(2c90b7f)</span></li>
<li>libs/editor/settings/infinibandwidget.cpp <span style="color: grey">(55dfb97)</span></li>
<li>libs/editor/settings/ipv4widget.h <span style="color: grey">(a993211)</span></li>
<li>libs/editor/settings/ipv4widget.cpp <span style="color: grey">(d7e893f)</span></li>
<li>libs/editor/settings/ipv6widget.h <span style="color: grey">(4a1f472)</span></li>
<li>libs/editor/settings/ipv6widget.cpp <span style="color: grey">(62e96ff)</span></li>
<li>libs/editor/settings/pppoewidget.h <span style="color: grey">(4570bba)</span></li>
<li>libs/editor/settings/pppoewidget.cpp <span style="color: grey">(0fa4ea2)</span></li>
<li>libs/editor/settings/pppwidget.h <span style="color: grey">(0c088ce)</span></li>
<li>libs/editor/settings/pppwidget.cpp <span style="color: grey">(7185b7d)</span></li>
<li>libs/editor/settings/security802-1x.h <span style="color: grey">(cb5dbec)</span></li>
<li>libs/editor/settings/security802-1x.cpp <span style="color: grey">(21929b8)</span></li>
<li>libs/editor/settings/teamwidget.h <span style="color: grey">(39821c2)</span></li>
<li>libs/editor/settings/teamwidget.cpp <span style="color: grey">(3db2b7a)</span></li>
<li>libs/editor/settings/ui/802-1x.ui <span style="color: grey">(f1bc058)</span></li>
<li>libs/editor/settings/ui/cdma.ui <span style="color: grey">(9805021)</span></li>
<li>libs/editor/settings/ui/gsm.ui <span style="color: grey">(87891cc)</span></li>
<li>libs/editor/settings/vlanwidget.h <span style="color: grey">(fabeaca)</span></li>
<li>libs/editor/settings/vlanwidget.cpp <span style="color: grey">(5e083fa)</span></li>
<li>libs/editor/settings/wificonnectionwidget.h <span style="color: grey">(35a59d3)</span></li>
<li>libs/editor/settings/wificonnectionwidget.cpp <span style="color: grey">(d35ec45)</span></li>
<li>libs/editor/settings/wifisecurity.h <span style="color: grey">(a263c32)</span></li>
<li>libs/editor/settings/wifisecurity.cpp <span style="color: grey">(4d9c811)</span></li>
<li>libs/editor/settings/wimaxwidget.h <span style="color: grey">(c8f850d)</span></li>
<li>libs/editor/settings/wimaxwidget.cpp <span style="color: grey">(dcd212c)</span></li>
<li>libs/editor/settings/wiredconnectionwidget.h <span style="color: grey">(ed8dc88)</span></li>
<li>libs/editor/settings/wiredconnectionwidget.cpp <span style="color: grey">(d8fcd90)</span></li>
<li>libs/editor/settings/wiredsecurity.h <span style="color: grey">(a34731e)</span></li>
<li>libs/editor/settings/wiredsecurity.cpp <span style="color: grey">(8fdb1cc)</span></li>
<li>libs/editor/widgets/passwordfield.h <span style="color: grey">(1bd21a2)</span></li>
<li>libs/editor/widgets/passwordfield.cpp <span style="color: grey">(05fe6dc)</span></li>
<li>libs/editor/widgets/settingwidget.h <span style="color: grey">(fdae197)</span></li>
<li>vpn/l2tp/l2tp.ui <span style="color: grey">(b04ebce)</span></li>
<li>vpn/l2tp/l2tpauth.h <span style="color: grey">(54b6462)</span></li>
<li>vpn/l2tp/l2tpauth.cpp <span style="color: grey">(7369458)</span></li>
<li>vpn/l2tp/l2tpwidget.h <span style="color: grey">(1e4c383)</span></li>
<li>vpn/l2tp/l2tpwidget.cpp <span style="color: grey">(b278228)</span></li>
<li>vpn/openconnect/openconnectauth.h <span style="color: grey">(9a77421)</span></li>
<li>vpn/openconnect/openconnectauth.cpp <span style="color: grey">(fecb16e)</span></li>
<li>vpn/openconnect/openconnectwidget.h <span style="color: grey">(ae0e9d2)</span></li>
<li>vpn/openconnect/openconnectwidget.cpp <span style="color: grey">(c293e52)</span></li>
<li>vpn/openswan/openswan.ui <span style="color: grey">(a6d61fa)</span></li>
<li>vpn/openswan/openswanauth.h <span style="color: grey">(aa78f88)</span></li>
<li>vpn/openswan/openswanauth.cpp <span style="color: grey">(1fd79fb)</span></li>
<li>vpn/openswan/openswanwidget.h <span style="color: grey">(55a54dd)</span></li>
<li>vpn/openswan/openswanwidget.cpp <span style="color: grey">(b94f752)</span></li>
<li>vpn/openvpn/openvpn.ui <span style="color: grey">(1c49cad)</span></li>
<li>vpn/openvpn/openvpnadvanced.ui <span style="color: grey">(ce89ce1)</span></li>
<li>vpn/openvpn/openvpnadvancedwidget.h <span style="color: grey">(c0346ee)</span></li>
<li>vpn/openvpn/openvpnadvancedwidget.cpp <span style="color: grey">(0864e35)</span></li>
<li>vpn/openvpn/openvpnauth.h <span style="color: grey">(dd3b564)</span></li>
<li>vpn/openvpn/openvpnauth.cpp <span style="color: grey">(5250bc1)</span></li>
<li>vpn/openvpn/openvpnwidget.h <span style="color: grey">(51d7aab)</span></li>
<li>vpn/openvpn/openvpnwidget.cpp <span style="color: grey">(e27e216)</span></li>
<li>vpn/pptp/pptpauth.h <span style="color: grey">(1772c81)</span></li>
<li>vpn/pptp/pptpauth.cpp <span style="color: grey">(8ffec08)</span></li>
<li>vpn/pptp/pptpprop.ui <span style="color: grey">(c713f24)</span></li>
<li>vpn/pptp/pptpwidget.h <span style="color: grey">(2b25dd7)</span></li>
<li>vpn/pptp/pptpwidget.cpp <span style="color: grey">(880d6c5)</span></li>
<li>vpn/ssh/sshauth.h <span style="color: grey">(88dcebc)</span></li>
<li>vpn/ssh/sshauth.cpp <span style="color: grey">(6e8ffa9)</span></li>
<li>vpn/ssh/sshwidget.h <span style="color: grey">(fa3f852)</span></li>
<li>vpn/ssh/sshwidget.cpp <span style="color: grey">(7b3ad28)</span></li>
<li>vpn/ssh/sshwidget.ui <span style="color: grey">(1d0300b)</span></li>
<li>vpn/sstp/sstpauth.h <span style="color: grey">(08f5025)</span></li>
<li>vpn/sstp/sstpauth.cpp <span style="color: grey">(b452a73)</span></li>
<li>vpn/sstp/sstpwidget.h <span style="color: grey">(0156b86)</span></li>
<li>vpn/sstp/sstpwidget.cpp <span style="color: grey">(7ff68aa)</span></li>
<li>vpn/sstp/sstpwidget.ui <span style="color: grey">(51dfc96)</span></li>
<li>vpn/strongswan/strongswanauth.h <span style="color: grey">(d23947d)</span></li>
<li>vpn/strongswan/strongswanauth.cpp <span style="color: grey">(f034abe)</span></li>
<li>vpn/strongswan/strongswanprop.ui <span style="color: grey">(06fb254)</span></li>
<li>vpn/strongswan/strongswanwidget.h <span style="color: grey">(7204ff5)</span></li>
<li>vpn/strongswan/strongswanwidget.cpp <span style="color: grey">(094e81e)</span></li>
<li>vpn/vpnc/vpnc.ui <span style="color: grey">(fc83bf8)</span></li>
<li>vpn/vpnc/vpncauth.h <span style="color: grey">(eb24744)</span></li>
<li>vpn/vpnc/vpncauth.cpp <span style="color: grey">(9142682)</span></li>
<li>vpn/vpnc/vpncwidget.h <span style="color: grey">(9aa4ac8)</span></li>
<li>vpn/vpnc/vpncwidget.cpp <span style="color: grey">(6aaffda)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125723/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>