Review Request 127172: PPTP authentication dialog: focus is now automatically set on "password" textbox.

Discussion:

Tomasz Bywalec

2016-02-24 23:05:29 UTC

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

Review request for Network Management.

Repository: plasma-nm

Description
-------

PPTP authentication dialog: focus is now automatically set on "password" textbox.

Also:

* The dialog's width is now the same as width of OpenVPN authentication dialog.
* "Show password" checkbox is now aligned to the left.

=================================================================================

Motivation for this change:

1. User experience: when keyboard focus is automatically set on "password" textbox, then user can start entering password immediately when the dialog is displayed.

2. Consistency of look&behaviour between PPTP and OpenVPN authentication dialogs.

Diffs
-----

vpn/pptp/pptpauth.cpp a2b1255
vpn/pptp/pptpauth.ui 86bbdb8

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

Testing
-------

Tested on Slackware64-current.

Thanks,

Tomasz Bywalec

Jan Grulich

2016-02-25 06:18:02 UTC

Permalink

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

Looks good, but branch 0.9.3 is already unmaintained. Is the same patch necessary in Plasma 5? I don't remember having this problem with OpenVPN/VPNC password dialogs and not sure right now if PPTP is different in this.

- Jan Grulich

Post by Tomasz Bywalec
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127172/
-----------------------------------------------------------
(Updated Ãno. 24, 2016, 11:05 odp.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
PPTP authentication dialog: focus is now automatically set on "password" textbox.
* The dialog's width is now the same as width of OpenVPN authentication dialog.
* "Show password" checkbox is now aligned to the left.
=================================================================================
1. User experience: when keyboard focus is automatically set on "password" textbox, then user can start entering password immediately when the dialog is displayed.
2. Consistency of look&behaviour between PPTP and OpenVPN authentication dialogs.
Diffs
-----
vpn/pptp/pptpauth.cpp a2b1255
vpn/pptp/pptpauth.ui 86bbdb8
Diff: https://git.reviewboard.kde.org/r/127172/diff/
Testing
-------
Tested on Slackware64-current.
Thanks,
Tomasz Bywalec

Tomasz Bywalec

2016-02-25 08:51:47 UTC

Permalink

Post by Tomasz Bywalec

Post by Jan Grulich
Looks good, but branch 0.9.3 is already unmaintained. Is the same patch necessary in Plasma 5? I don't remember having this problem with OpenVPN/VPNC password dialogs and not sure right now if PPTP is different in this.

Hi Jan

I've checked this patch only with version 0.9.3.6, because this is the version of plasma-nm which is shipped with the latest version of Slackware64-current, which I use on an everyday basis. I will try to check this with version 5 and will get back to you.

- Tomasz

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

Post by Tomasz Bywalec
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127172/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 11:05 p.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
PPTP authentication dialog: focus is now automatically set on "password" textbox.
* The dialog's width is now the same as width of OpenVPN authentication dialog.
* "Show password" checkbox is now aligned to the left.
=================================================================================
1. User experience: when keyboard focus is automatically set on "password" textbox, then user can start entering password immediately when the dialog is displayed.
2. Consistency of look&behaviour between PPTP and OpenVPN authentication dialogs.
Diffs
-----
vpn/pptp/pptpauth.cpp a2b1255
vpn/pptp/pptpauth.ui 86bbdb8
Diff: https://git.reviewboard.kde.org/r/127172/diff/
Testing
-------
Tested on Slackware64-current.
Thanks,
Tomasz Bywalec

Tomasz Bywalec

2016-03-01 08:44:46 UTC

Permalink

Post by Tomasz Bywalec

Hi Jan

Apologies for late reply. I've checked this with version 5.5.4, and it looks like the mentioned issue doesn't occur there. The only other GNU/Linux distribution shipped with KDE 4, which I've managed to verify, is ROSA Desktop Fresh KDE R7 - like Slackware(64)-current, it too uses plasma-nm 0.9.3.?, and the same UI issue occurs there.

In other words, it looks like there is no need to prepare a similar patch for Plasma 5. On the other hand, the patch attached in this request could potentially benefit users of KDE 4 (I'm not sure if plasma5-nm can be used in KDE 4 ?).

- Tomasz

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

Post by Tomasz Bywalec
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127172/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 11:05 p.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
PPTP authentication dialog: focus is now automatically set on "password" textbox.
* The dialog's width is now the same as width of OpenVPN authentication dialog.
* "Show password" checkbox is now aligned to the left.
=================================================================================
1. User experience: when keyboard focus is automatically set on "password" textbox, then user can start entering password immediately when the dialog is displayed.
2. Consistency of look&behaviour between PPTP and OpenVPN authentication dialogs.
Diffs
-----
vpn/pptp/pptpauth.cpp a2b1255
vpn/pptp/pptpauth.ui 86bbdb8
Diff: https://git.reviewboard.kde.org/r/127172/diff/
Testing
-------
Tested on Slackware64-current.
Thanks,
Tomasz Bywalec

Jan Grulich

2016-03-01 09:10:34 UTC

Permalink

Post by Tomasz Bywalec

Hi Jan
I've checked this patch only with version 0.9.3.6, because this is the version of plasma-nm which is shipped with the latest version of Slackware64-current, which I use on an everyday basis. I will try to check this with version 5 and will get back to you.
Hi Jan
Apologies for late reply. I've checked this with version 5.5.4, and it looks like the mentioned issue doesn't occur there. The only other GNU/Linux distribution shipped with KDE 4, which I've managed to verify, is ROSA Desktop Fresh KDE R7 - like Slackware(64)-current, it too uses plasma-nm 0.9.3.?, and the same UI issue occurs there.
In other words, it looks like there is no need to prepare a similar patch for Plasma 5. On the other hand, the patch attached in this request could potentially benefit users of KDE 4 (I'm not sure if plasma5-nm can be used in KDE 4 ?).

Plasma-nm from Plasma 5 cannot be used in KDE 4 unfortunately. You can push your patch, but I doubt it will help to someone, KDE 4 is now unmaintained and there won't be any new release of it.

- Jan

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

Jan Grulich

2016-03-01 09:10:39 UTC

Permalink

Tomasz Bywalec

2016-03-05 15:22:31 UTC

Permalink

Post by Tomasz Bywalec

Post by Jan Grulich
Ship It!

I'm afraid I don't have commit access to KDE git repository, and it seems I have too short contribution history (1 patch :-) to be granted such access. It was suggested to me that perhaps someone else, who has the access, can commit the patch for me. Would it be possible for someone working on plasma-nm to do this? Apologies if it causes any problem.

- Tomasz

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

Post by Tomasz Bywalec
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127172/
-----------------------------------------------------------
(Updated Ð€ÐµÐ². 24, 2016, 11:05 Ð¿.Ð¿.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
PPTP authentication dialog: focus is now automatically set on "password" textbox.
* The dialog's width is now the same as width of OpenVPN authentication dialog.
* "Show password" checkbox is now aligned to the left.
=================================================================================
1. User experience: when keyboard focus is automatically set on "password" textbox, then user can start entering password immediately when the dialog is displayed.
2. Consistency of look&behaviour between PPTP and OpenVPN authentication dialogs.
Diffs
-----
vpn/pptp/pptpauth.cpp a2b1255
vpn/pptp/pptpauth.ui 86bbdb8
Diff: https://git.reviewboard.kde.org/r/127172/diff/
Testing
-------
Tested on Slackware64-current.
Thanks,
Tomasz Bywalec

Tomasz Bywalec

2016-03-05 15:34:40 UTC

Permalink

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

(Updated March 5, 2016, 3:34 p.m.)

Status
------

This change has been marked as submitted.

Review request for Network Management.

Changes
-------

Submitted with commit ccc43a548c62d8aa2031446bfb23bd294b0b4d97 by Lamarque V. Souza on behalf of Tomasz Bywalec to branch 0.9.3.

Repository: plasma-nm

Description
-------

PPTP authentication dialog: focus is now automatically set on "password" textbox.

Also:

* The dialog's width is now the same as width of OpenVPN authentication dialog.
* "Show password" checkbox is now aligned to the left.

=================================================================================

Motivation for this change:

1. User experience: when keyboard focus is automatically set on "password" textbox, then user can start entering password immediately when the dialog is displayed.

2. Consistency of look&behaviour between PPTP and OpenVPN authentication dialogs.

Diffs
-----

vpn/pptp/pptpauth.cpp a2b1255
vpn/pptp/pptpauth.ui 86bbdb8

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

Testing
-------

Tested on Slackware64-current.

Thanks,

Tomasz Bywalec