'Add SSH Key' Improvements #1802
Conversation
|
@m3nu @real-yfprojects What do you think of these changes? Is this a better experience for the user than the current behavior? For now, the these changes break a couple tests. If we like the changes then I will be happy to refactor those tests accordingly. |
|
Shouldn't it close after the key is added. But yes, the SSH stuff can be a barrier for new users, so any improvement is welcome. Still need to try this locally. |
Yes, I believe it should. However, in the current implementation it does not
Please do - I'd appreciate hearing how to further improve it. |
I don't think we need that since the button already states what is going to happen. |
Good point. Maybe only show a message if the key creation fails? |
|
Sounds reasonable 👍 |
|
@real-yfprojects I made those changes. Here is a demo of the latest changes below. What do you think? With your approval of the final design, I will refactor the tests 👍
|
src/vorta/views/ssh_dialog.py
Outdated
| @@ -11,6 +11,8 @@ | |||
|
|
|||
|
|
|||
| class SSHAddWindow(SSHAddBase, SSHAddUI): | |||
| create_ssh_key_failure = QtCore.pyqtSignal(int) | |||
There was a problem hiding this comment.
| create_ssh_key_failure = QtCore.pyqtSignal(int) | |
| failure = QtCore.pyqtSignal(int) |
| @@ -68,15 +70,16 @@ def generate_key(self): | |||
| self.sshproc.finished.connect(self.generate_key_result) | |||
| self.sshproc.start('ssh-keygen', ['-t', format, '-b', length, '-f', output_path, '-N', '']) | |||
|
|
|||
| def generate_key_result(self, exitCode, exitStatus): | |||
| if exitCode == 0: | |||
| def generate_key_result(self, exit_code): | |||
There was a problem hiding this comment.
| def generate_key_result(self, exit_code): | |
| @pyqtSlot(int) | |
| def generate_key_result(self, exit_code): |
Not sure whether that is needed. But it might be since the finished signal can be emitted with two values.
|
@real-yfprojects @m3nu I included the changes for the signal, and added tests for a ssh key add success and failure. PR is ready for review 👍 |
|
Thanks, @bigtedde! Works as promised locally. 👏 |
Description
Issue described here: #1801
In addition to fixing the issue of adding keys to the dropdown right away, I tweaked the process for adding a key such that now when the user adds a new key, they receive a QMessageBox alert informing them and the SSH dialogue box closes. Previously, the SSH dialogue box would not close unless the user clicked 'cancel', which seems counterintuitive. Here is a demo of the changes:
Related Issue
#1801
Motivation and Context
Greater consistency and (hopefully) improved user experience.
How Has This Been Tested?
Locally
Types of changes
Checklist:
I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.