Review Request 129212: Move password storing options into a separated combobox

Discussion:

Jan Grulich

2016-10-18 06:01:57 UTC

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

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

Repository: plasma-nm

Description
-------

As discussed over email, currently the password options (Save, Not Required, Always ask) are not easy to find so we decided to move them back to separated combobox again, ecept moving the combobox below the password field with a better description.

![Example of wireless setting](Loading Image...

)

Diffs
-----

libs/editor/settings/cdmawidget.cpp 309b6e6
libs/editor/settings/gsmwidget.cpp de5a7ef
libs/editor/settings/pppoewidget.cpp 1305797
libs/editor/settings/security802-1x.cpp e5f0307
libs/editor/settings/wifisecurity.cpp fc607eb
libs/editor/widgets/passwordfield.h 627d9b0
libs/editor/widgets/passwordfield.cpp 68aa3b8
vpn/openvpn/openvpnadvancedwidget.cpp 1dea814
vpn/openvpn/openvpnwidget.cpp 5241883
vpn/pptp/pptpwidget.cpp dabaa25
vpn/sstp/sstpwidget.cpp f0bf10a
vpn/vpnc/vpncwidget.cpp 7c547f9

Diff: https://git.reviewboard.kde.org/r/129212/diff/

Testing
-------

Thanks,

Jan Grulich

Anthony Fieroni

2016-10-18 06:32:34 UTC

Permalink

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

libs/editor/widgets/passwordfield.h (line 24)
<https://git.reviewboard.kde.org/r/129212/#comment67234>

KLineEdit is deprecated for QLineEdit

libs/editor/widgets/passwordfield.h (line 25)
<https://git.reviewboard.kde.org/r/129212/#comment67235>

QComboBox ?

- Anthony Fieroni

Jan Grulich

2016-10-18 06:40:59 UTC

Permalink

Albert Astals Cid

2016-10-18 11:29:19 UTC

Permalink

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

Looks good to me, I'd "Ship It" it but since there's lots of people in the list of CC, let's let them give their opinion :)

- Albert Astals Cid

Lamarque Souza

2016-10-18 12:29:58 UTC

Permalink

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

Fix it, then Ship it!

libs/editor/widgets/passwordfield.h (line 45)
<https://git.reviewboard.kde.org/r/129212/#comment67248>

The parameter name is "enable", not "enabled".

libs/editor/widgets/passwordfield.cpp (line 76)
<https://git.reviewboard.kde.org/r/129212/#comment67246>

This should be setPasswordModeEnabled(bool enable)

libs/editor/widgets/passwordfield.cpp (line 107)
<https://git.reviewboard.kde.org/r/129212/#comment67247>

The & goes next the variable name:

const QString &text

- Lamarque Souza

Jan Grulich

2016-10-18 13:37:10 UTC

Permalink

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

(Updated ÅÃj. 18, 2016, 1:37 odp.)

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

Changes
-------

Updating with fixes for mentioned issues and waiting for usability group to approve this.

Repository: plasma-nm

Description
-------

As discussed over email, currently the password options (Save, Not Required, Always ask) are not easy to find so we decided to move them back to separated combobox again, ecept moving the combobox below the password field with a better description.

![Example of wireless setting](https://jgrulich.fedorapeople.org/password-field-new.png)

Diffs (updated)
-----

kded/pinwidget.ui 61c95c0
libs/editor/settings/cdmawidget.cpp 309b6e6
libs/editor/settings/gsmwidget.cpp de5a7ef
libs/editor/settings/pppoewidget.cpp 1305797
libs/editor/settings/security802-1x.cpp e5f0307
libs/editor/settings/ui/802-1x.ui 54ae4fc
libs/editor/settings/ui/cdma.ui bace7ce
libs/editor/settings/ui/gsm.ui e1c729b
libs/editor/settings/ui/pppoe.ui bc6ee35
libs/editor/settings/ui/wifisecurity.ui 27610d0
libs/editor/settings/wifisecurity.cpp fc607eb
libs/editor/widgets/passwordfield.h 627d9b0
libs/editor/widgets/passwordfield.cpp 68aa3b8
vpn/iodine/iodineauth.ui 1dcee23
vpn/l2tp/l2tp.ui 3dedc44
vpn/l2tp/l2tpauth.ui a5de7fe
vpn/openconnect/openconnectauth.cpp 1602fb8
vpn/openswan/openswan.ui b5e107d
vpn/openswan/openswanauth.ui aa7f626
vpn/openvpn/openvpn.ui f95e466
vpn/openvpn/openvpnadvanced.ui 24f7564
vpn/openvpn/openvpnadvancedwidget.cpp 1dea814
vpn/openvpn/openvpnauth.cpp c0d2a46
vpn/openvpn/openvpnwidget.cpp 5241883
vpn/pptp/pptpauth.ui e77a480
vpn/pptp/pptpprop.ui 33c1e86
vpn/pptp/pptpwidget.cpp dabaa25
vpn/ssh/sshauth.ui 1e4bf62
vpn/ssh/sshwidget.ui 95b4b6d
vpn/sstp/sstpadvanced.ui d198620
vpn/sstp/sstpauth.ui 2fad484
vpn/sstp/sstpwidget.cpp f0bf10a
vpn/sstp/sstpwidget.ui c4a0b42
vpn/strongswan/strongswanauth.ui d605290
vpn/strongswan/strongswanprop.ui 9ce7376
vpn/vpnc/vpnc.ui ab0ba36
vpn/vpnc/vpncauth.ui 22a3e4e
vpn/vpnc/vpncwidget.cpp 7c547f9

Diff: https://git.reviewboard.kde.org/r/129212/diff/

Testing
-------

Thanks,

Jan Grulich

Heiko Tietze

2016-10-20 08:19:32 UTC

Permalink

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

Ship it!

Well done. Was able to prove myself wrong about first concerns regarding text length exceeding the dropdown width; bracketed text is most often the second best solution. Perhaps have a closer look on the alignment that might be not perfect on the right (wonder how it looks on resize). Consider to either indent further on the left side so password is aligned with the dropdown above, or just anchor it to the left without indention. Both should work well.

- Heiko Tietze

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129212/
-----------------------------------------------------------
(Updated Oct. 18, 2016, 1:37 p.m.)
Review request for Network Management, KDE Usability, Albert Astals Cid, and Lamarque Souza.
Repository: plasma-nm
Description
-------
As discussed over email, currently the password options (Save, Not Required, Always ask) are not easy to find so we decided to move them back to separated combobox again, ecept moving the combobox below the password field with a better description.
![Example of wireless setting](https://jgrulich.fedorapeople.org/password-field-new.png)
Diffs
-----
kded/pinwidget.ui 61c95c0
libs/editor/settings/cdmawidget.cpp 309b6e6
libs/editor/settings/gsmwidget.cpp de5a7ef
libs/editor/settings/pppoewidget.cpp 1305797
libs/editor/settings/security802-1x.cpp e5f0307
libs/editor/settings/ui/802-1x.ui 54ae4fc
libs/editor/settings/ui/cdma.ui bace7ce
libs/editor/settings/ui/gsm.ui e1c729b
libs/editor/settings/ui/pppoe.ui bc6ee35
libs/editor/settings/ui/wifisecurity.ui 27610d0
libs/editor/settings/wifisecurity.cpp fc607eb
libs/editor/widgets/passwordfield.h 627d9b0
libs/editor/widgets/passwordfield.cpp 68aa3b8
vpn/iodine/iodineauth.ui 1dcee23
vpn/l2tp/l2tp.ui 3dedc44
vpn/l2tp/l2tpauth.ui a5de7fe
vpn/openconnect/openconnectauth.cpp 1602fb8
vpn/openswan/openswan.ui b5e107d
vpn/openswan/openswanauth.ui aa7f626
vpn/openvpn/openvpn.ui f95e466
vpn/openvpn/openvpnadvanced.ui 24f7564
vpn/openvpn/openvpnadvancedwidget.cpp 1dea814
vpn/openvpn/openvpnauth.cpp c0d2a46
vpn/openvpn/openvpnwidget.cpp 5241883
vpn/pptp/pptpauth.ui e77a480
vpn/pptp/pptpprop.ui 33c1e86
vpn/pptp/pptpwidget.cpp dabaa25
vpn/ssh/sshauth.ui 1e4bf62
vpn/ssh/sshwidget.ui 95b4b6d
vpn/sstp/sstpadvanced.ui d198620
vpn/sstp/sstpauth.ui 2fad484
vpn/sstp/sstpwidget.cpp f0bf10a
vpn/sstp/sstpwidget.ui c4a0b42
vpn/strongswan/strongswanauth.ui d605290
vpn/strongswan/strongswanprop.ui 9ce7376
vpn/vpnc/vpnc.ui ab0ba36
vpn/vpnc/vpncauth.ui 22a3e4e
vpn/vpnc/vpncwidget.cpp 7c547f9
Diff: https://git.reviewboard.kde.org/r/129212/diff/
Testing
-------
Thanks,
Jan Grulich

Jan Grulich

2016-10-20 09:41:42 UTC

Permalink

Post by Jan Grulich

Post by Heiko Tietze
Well done. Was able to prove myself wrong about first concerns regarding text length exceeding the dropdown width; bracketed text is most often the second best solution. Perhaps have a closer look on the alignment that might be not perfect on the right (wonder how it looks on resize). Consider to either indent further on the left side so password is aligned with the dropdown above, or just anchor it to the left without indention. Both should work well.

I'm not sure I can do anything about the alignment, because it's a QStackedWidget where we have nested various widget combinations based on security type. But I'll check that.

- Jan

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

Post by Jan Grulich
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129212/
-----------------------------------------------------------
(Updated ÅÃj. 18, 2016, 1:37 odp.)
Review request for Network Management, KDE Usability, Albert Astals Cid, and Lamarque Souza.
Repository: plasma-nm
Description
-------
As discussed over email, currently the password options (Save, Not Required, Always ask) are not easy to find so we decided to move them back to separated combobox again, ecept moving the combobox below the password field with a better description.
![Example of wireless setting](https://jgrulich.fedorapeople.org/password-field-new.png)
Diffs
-----
kded/pinwidget.ui 61c95c0
libs/editor/settings/cdmawidget.cpp 309b6e6
libs/editor/settings/gsmwidget.cpp de5a7ef
libs/editor/settings/pppoewidget.cpp 1305797
libs/editor/settings/security802-1x.cpp e5f0307
libs/editor/settings/ui/802-1x.ui 54ae4fc
libs/editor/settings/ui/cdma.ui bace7ce
libs/editor/settings/ui/gsm.ui e1c729b
libs/editor/settings/ui/pppoe.ui bc6ee35
libs/editor/settings/ui/wifisecurity.ui 27610d0
libs/editor/settings/wifisecurity.cpp fc607eb
libs/editor/widgets/passwordfield.h 627d9b0
libs/editor/widgets/passwordfield.cpp 68aa3b8
vpn/iodine/iodineauth.ui 1dcee23
vpn/l2tp/l2tp.ui 3dedc44
vpn/l2tp/l2tpauth.ui a5de7fe
vpn/openconnect/openconnectauth.cpp 1602fb8
vpn/openswan/openswan.ui b5e107d
vpn/openswan/openswanauth.ui aa7f626
vpn/openvpn/openvpn.ui f95e466
vpn/openvpn/openvpnadvanced.ui 24f7564
vpn/openvpn/openvpnadvancedwidget.cpp 1dea814
vpn/openvpn/openvpnauth.cpp c0d2a46
vpn/openvpn/openvpnwidget.cpp 5241883
vpn/pptp/pptpauth.ui e77a480
vpn/pptp/pptpprop.ui 33c1e86
vpn/pptp/pptpwidget.cpp dabaa25
vpn/ssh/sshauth.ui 1e4bf62
vpn/ssh/sshwidget.ui 95b4b6d
vpn/sstp/sstpadvanced.ui d198620
vpn/sstp/sstpauth.ui 2fad484
vpn/sstp/sstpwidget.cpp f0bf10a
vpn/sstp/sstpwidget.ui c4a0b42
vpn/strongswan/strongswanauth.ui d605290
vpn/strongswan/strongswanprop.ui 9ce7376
vpn/vpnc/vpnc.ui ab0ba36
vpn/vpnc/vpncauth.ui 22a3e4e
vpn/vpnc/vpncwidget.cpp 7c547f9
Diff: https://git.reviewboard.kde.org/r/129212/diff/
Testing
-------
Thanks,
Jan Grulich

Jan Grulich

2016-10-20 09:53:10 UTC

Permalink

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

(Updated Oct. 20, 2016, 9:53 a.m.)

Status
------

This change has been marked as submitted.

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

Changes
-------

Submitted with commit 3a3a21e3846c95433de228957804b91bcd76c8fd by Jan Grulich to branch master.

Repository: plasma-nm

Description
-------

As discussed over email, currently the password options (Save, Not Required, Always ask) are not easy to find so we decided to move them back to separated combobox again, ecept moving the combobox below the password field with a better description.

![Example of wireless setting](https://jgrulich.fedorapeople.org/password-field-new.png)

Diffs
-----

kded/pinwidget.ui 61c95c0
libs/editor/settings/cdmawidget.cpp 309b6e6
libs/editor/settings/gsmwidget.cpp de5a7ef
libs/editor/settings/pppoewidget.cpp 1305797
libs/editor/settings/security802-1x.cpp e5f0307
libs/editor/settings/ui/802-1x.ui 54ae4fc
libs/editor/settings/ui/cdma.ui bace7ce
libs/editor/settings/ui/gsm.ui e1c729b
libs/editor/settings/ui/pppoe.ui bc6ee35
libs/editor/settings/ui/wifisecurity.ui 27610d0
libs/editor/settings/wifisecurity.cpp fc607eb
libs/editor/widgets/passwordfield.h 627d9b0
libs/editor/widgets/passwordfield.cpp 68aa3b8
vpn/iodine/iodineauth.ui 1dcee23
vpn/l2tp/l2tp.ui 3dedc44
vpn/l2tp/l2tpauth.ui a5de7fe
vpn/openconnect/openconnectauth.cpp 1602fb8
vpn/openswan/openswan.ui b5e107d
vpn/openswan/openswanauth.ui aa7f626
vpn/openvpn/openvpn.ui f95e466
vpn/openvpn/openvpnadvanced.ui 24f7564
vpn/openvpn/openvpnadvancedwidget.cpp 1dea814
vpn/openvpn/openvpnauth.cpp c0d2a46
vpn/openvpn/openvpnwidget.cpp 5241883
vpn/pptp/pptpauth.ui e77a480
vpn/pptp/pptpprop.ui 33c1e86
vpn/pptp/pptpwidget.cpp dabaa25
vpn/ssh/sshauth.ui 1e4bf62
vpn/ssh/sshwidget.ui 95b4b6d
vpn/sstp/sstpadvanced.ui d198620
vpn/sstp/sstpauth.ui 2fad484
vpn/sstp/sstpwidget.cpp f0bf10a
vpn/sstp/sstpwidget.ui c4a0b42
vpn/strongswan/strongswanauth.ui d605290
vpn/strongswan/strongswanprop.ui 9ce7376
vpn/vpnc/vpnc.ui ab0ba36
vpn/vpnc/vpncauth.ui 22a3e4e
vpn/vpnc/vpncwidget.cpp 7c547f9

Diff: https://git.reviewboard.kde.org/r/129212/diff/

Testing
-------

Thanks,

Jan Grulich