Review Request 129221: Minor improvements to the editor UI

[Jan Grulich]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------

Review request for Network Management 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

[Lamarque Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100135
-----------------------------------------------------------



Well, I cannot see most of the changes now because I do not have time to recompile Plasma NM. If you can add screenshots of the changes would help me a low review the changes.


libs/editor/settings/ui/ipv6.ui (line 29)
<https://git.reviewboard.kde.org/r/129221/#comment67257>

I think the "When connecting to IPv5-capable networks," is not needed. The "IPv4 configuration succeeds" part implies that this setting has effect only when IPv4 is enabled.



libs/editor/settings/ui/ipv6.ui (line 83)
<https://git.reviewboard.kde.org/r/129221/#comment67258>

Edit list of domain name servers.



libs/editor/settings/ui/ipv6.ui (line 194)
<https://git.reviewboard.kde.org/r/129221/#comment67259>

The ", on the other hand" part sounds strange. I think it can be omitted.



libs/editor/settings/ui/wificonnectionwidget.ui (line 130)
<https://git.reviewboard.kde.org/r/129221/#comment67256>

This tooltip is not accurate. The wifi network is identified by the ssid and may comprise several access points. This option fixes one of those access points so the wifi card does not jump from one access point to another *in the same network*.

As far as I know someone can have two or more access points in the same network (same ssid) with the same frequency band (same channel). This setting does not seem band specific to me, so the change "network associated to the specified band" does not seem accurate to me.



vpn/vpnc/vpncadvanced.ui (line 209)
<https://git.reviewboard.kde.org/r/129221/#comment67260>

s/sets/set/


- Lamarque Souza

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------
(Updated Oct. 19, 2016, 11:03 a.m.)
Review request for Network Management 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

[Jan Grulich]

Post by Jan Grulich

libs/editor/settings/ui/wificonnectionwidget.ui, line 133
<https://git.reviewboard.kde.org/r/129221/diff/1/?file=482590#file482590line133>
This tooltip is not accurate. The wifi network is identified by the ssid and may comprise several access points. This option fixes one of those access points so the wifi card does not jump from one access point to another *in the same network*.
As far as I know someone can have two or more access points in the same network (same ssid) with the same frequency band (same channel). This setting does not seem band specific to me, so the change "network associated to the specified band" does not seem accurate to me.

It's correct, this tooltip is for BSSID, which allows you to force connecting to a certain AP.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100135
-----------------------------------------------------------

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------
(Updated Říj. 19, 2016, 11:03 dop.)
Review request for Network Management 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

[Jan Grulich]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------

(Updated Říj. 20, 2016, 5:43 dop.)


Review request for Network Management and Lamarque Souza.


Changes
-------

Fixes tooltips.

Images for comparison:


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 (updated)
-----

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

[Heiko Tietze]

-----------------------------------------------------------
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

Post by Jan Grulich
-----------------------------------------------------------
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

[Jan Grulich]

Post by Jan Grulich

Post by Heiko Tietze
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.

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.

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.

We can remove colons later if we agree on that.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100159
-----------------------------------------------------------

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------
(Updated Říj. 20, 2016, 8:13 dop.)
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

[Heiko Tietze]

Post by Jan Grulich

Post by Heiko Tietze
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.

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.
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.
We can remove colons later if we agree on that.

Oops, old/new... Ship it.


- Heiko


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100159
-----------------------------------------------------------

Post by Jan Grulich
-----------------------------------------------------------
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

[Heiko Tietze]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100165
-----------------------------------------------------------


Ship it!




Ship It!

- Heiko Tietze

Post by Jan Grulich
-----------------------------------------------------------
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

[Lamarque Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100172
-----------------------------------------------------------


Fix it, then Ship it!





libs/editor/settings/ui/wificonnectionwidget.ui (line 178)
<https://git.reviewboard.kde.org/r/129221/#comment67277>

This label looks misaligned in screenshot 4. I mean, the distance between BSSID and "Restrict to device" labels is bigger than in the other cases.


- Lamarque Souza

Post by Jan Grulich
-----------------------------------------------------------
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

[Jan Grulich]

Post by Jan Grulich

libs/editor/settings/ui/wificonnectionwidget.ui, line 181
<https://git.reviewboard.kde.org/r/129221/diff/2/?file=482712#file482712line181>
This label looks misaligned in screenshot 4. I mean, the distance between BSSID and "Restrict to device" labels is bigger than in the other cases.

That's because there are two hidden widgets, for setting band and channel, but these are hidden when the mode is set to infrastructure. I couldn't find a way how to remove the big space, even setting widget height to 0 doesn't help. I can do what I did with the IPv4 configuration and make all widgets visible, but just disabled.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100172
-----------------------------------------------------------

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------
(Updated Říj. 20, 2016, 8:13 dop.)
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

[Lamarque Souza]

Post by Jan Grulich

libs/editor/settings/ui/wificonnectionwidget.ui, line 181
<https://git.reviewboard.kde.org/r/129221/diff/2/?file=482712#file482712line181>
This label looks misaligned in screenshot 4. I mean, the distance between BSSID and "Restrict to device" labels is bigger than in the other cases.

That's because there are two hidden widgets, for setting band and channel, but these are hidden when the mode is set to infrastructure. I couldn't find a way how to remove the big space, even setting widget height to 0 doesn't help. I can do what I did with the IPv4 configuration and make all widgets visible, but just disabled.

I thought that would be the case. The default spacing between widgets in a QLayout is 4px, that probably explain the space between those labels. Well, that space is the same for all rows, I do not know how to solve that without adding too much code to manage the spacing between widgets in the layout ourselves.


- Lamarque


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/#review100172
-----------------------------------------------------------

Post by Jan Grulich
-----------------------------------------------------------
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

[Jan Grulich]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129221/
-----------------------------------------------------------

(Updated Oct. 20, 2016, 1:24 p.m.)


Status
------

This change has been marked as submitted.


Review request for Network Management, KDE Usability and Lamarque Souza.


Changes
-------

Submitted with commit 7438321bab2995b512e35104c03c87d2eeefa4d5 by Jan Grulich to branch master.


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