----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129221/#review100159 -----------------------------------------------------------
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. 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. "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 (" 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). 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. - Heiko Tietze On Oct. 20, 2016, 8:13 a.m., Jan Grulich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129221/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2016, 8:13 a.m.) > > > Review request for Network Management, KDE Usability and Lamarque Souza. > > > Repository: plasma-nm > > > Description > ------- > > + vertical spacing between all widgets set to 6px (previously only few of > them had this spacing) > + checkbox alignment according to HIG > + correct label alignment for some VPN widgets > > > Diffs > ----- > > libs/editor/settings/ipv4widget.cpp 2e92061 > libs/editor/settings/ipv6widget.cpp c5fa3b8 > libs/editor/settings/ui/802-1x.ui 54ae4fc > libs/editor/settings/ui/bond.ui 1c7d1c3 > libs/editor/settings/ui/bridge.ui 61a1db6 > libs/editor/settings/ui/bt.ui 295cd9e > libs/editor/settings/ui/cdma.ui bace7ce > libs/editor/settings/ui/infiniband.ui a3ab450 > libs/editor/settings/ui/ipv4.ui 0aae15e > libs/editor/settings/ui/ipv6.ui 58d0c89 > libs/editor/settings/ui/ppp.ui f53f38b > libs/editor/settings/ui/pppoe.ui bc6ee35 > libs/editor/settings/ui/vlan.ui 1ea5bae > libs/editor/settings/ui/wificonnectionwidget.ui 283cff4 > libs/editor/settings/ui/wifisecurity.ui 27610d0 > libs/editor/settings/ui/wiredconnectionwidget.ui c07ae7f > vpn/iodine/iodine.ui ac92c17 > vpn/l2tp/l2tp.ui 3dedc44 > vpn/l2tp/l2tpadvanced.ui 80e333f > vpn/l2tp/l2tpauth.ui a5de7fe > vpn/l2tp/l2tpppp.ui 64ce2ac > vpn/openconnect/openconnectprop.ui ed7dfde > vpn/openswan/openswan.ui b5e107d > vpn/openswan/openswanauth.ui aa7f626 > vpn/openvpn/openvpnadvanced.ui 24f7564 > vpn/pptp/pptpadvanced.ui dd51bc7 > vpn/pptp/pptpprop.ui 33c1e86 > vpn/ssh/sshwidget.ui 95b4b6d > vpn/sstp/sstpadvanced.ui d198620 > vpn/sstp/sstpwidget.ui c4a0b42 > vpn/vpnc/vpnc.ui ab0ba36 > vpn/vpnc/vpncadvanced.ui 9c27f85 > > Diff: https://git.reviewboard.kde.org/r/129221/diff/ > > > Testing > ------- > > > Thanks, > > Jan Grulich > >
