New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using compact settings for RDP plugin closes Issue 757 #759

Merged
merged 4 commits into from Feb 3, 2016

Conversation

Projects
None yet
3 participants
@antenore
Member

antenore commented Feb 3, 2016

This pull request improve the Advanced tab check boxes placement to reduce the height of the notebook.

advancetab_compact-view

@antenore

This comment has been minimized.

Show comment
Hide comment
@antenore

antenore Feb 3, 2016

Member

@giox069 , @weberhofer if you have nothing against this I'm going to merge it.

I'll keep the branch as I want to explore other solutions, like an horizontal notebook, instead of the vertical one we have.

I'd like for example to put the profile description on the left and the settings (basic, advanced and SSH) on the right

Member

antenore commented Feb 3, 2016

@giox069 , @weberhofer if you have nothing against this I'm going to merge it.

I'll keep the branch as I want to explore other solutions, like an horizontal notebook, instead of the vertical one we have.

I'd like for example to put the profile description on the left and the settings (basic, advanced and SSH) on the right

@weberhofer

This comment has been minimized.

Show comment
Hide comment
@weberhofer

weberhofer Feb 3, 2016

Collaborator

Looks very good; regarding the usability, it could be even better to group the checkboxes by functionality: You could at the left side have

  • Ignore cert
  • Share Smartcard
  • Disable password storing
    and the other points, and on the right side
  • Redirect mic
  • Share local printers
  • Disable clipboard sync

From the users point of view, I would (if possible), invert the "Disable [password storing|clipboard sync]" features and per default check them. Deactivating something by activating it is always hard to understand.

Collaborator

weberhofer commented Feb 3, 2016

Looks very good; regarding the usability, it could be even better to group the checkboxes by functionality: You could at the left side have

  • Ignore cert
  • Share Smartcard
  • Disable password storing
    and the other points, and on the right side
  • Redirect mic
  • Share local printers
  • Disable clipboard sync

From the users point of view, I would (if possible), invert the "Disable [password storing|clipboard sync]" features and per default check them. Deactivating something by activating it is always hard to understand.

@giox069

This comment has been minimized.

Show comment
Hide comment
@giox069

giox069 Feb 3, 2016

Contributor

Looks good. It now fits my old laptop display (1280x800).
I agree, if possible, to change "disable clipboard sync" with "enable clipboard sync" and set it enabled by default in new profiles. Unfortunately we will lose string translations in .po files... but, no problems.

I'm not sure with "Disable password storing" because it was a specific request to have this checbox, a kind of "extra security feature" that the user want explicitly to see.

Ordering: it seems you mean that certificate, password, smartcard all belong to "authentication" and should go together ? Ok, it can be sorted in that way :)

Contributor

giox069 commented Feb 3, 2016

Looks good. It now fits my old laptop display (1280x800).
I agree, if possible, to change "disable clipboard sync" with "enable clipboard sync" and set it enabled by default in new profiles. Unfortunately we will lose string translations in .po files... but, no problems.

I'm not sure with "Disable password storing" because it was a specific request to have this checbox, a kind of "extra security feature" that the user want explicitly to see.

Ordering: it seems you mean that certificate, password, smartcard all belong to "authentication" and should go together ? Ok, it can be sorted in that way :)

@antenore

This comment has been minimized.

Show comment
Hide comment
@antenore

antenore Feb 3, 2016

Member

Done regarding the order. For the other two options, it's quite cumbersome to make it works correctly.

It's also a bit out of scope compared to this specific issue.

I'm trying to rethink completely the remmina_file_editor and I'll keep in mind these inputs as well.

On the other side, if you have time to improve it go ahead.

I merge this.

Member

antenore commented Feb 3, 2016

Done regarding the order. For the other two options, it's quite cumbersome to make it works correctly.

It's also a bit out of scope compared to this specific issue.

I'm trying to rethink completely the remmina_file_editor and I'll keep in mind these inputs as well.

On the other side, if you have time to improve it go ahead.

I merge this.

antenore added a commit that referenced this pull request Feb 3, 2016

Merge pull request #759 from FreeRDP/issue_757-compactset
Using compact settings for RDP plugin closes Issue 757

@antenore antenore merged commit 2a89631 into next Feb 3, 2016

@weberhofer

This comment has been minimized.

Show comment
Hide comment
@weberhofer

weberhofer Feb 4, 2016

Collaborator

Looks good! I can imagine that including a reverse-logic is much work but it might be good to keep it in mind.

Collaborator

weberhofer commented Feb 4, 2016

Looks good! I can imagine that including a reverse-logic is much work but it might be good to keep it in mind.

@antenore antenore deleted the issue_757-compactset branch Feb 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment