Skip to content
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

[keybindings] add 'Reset' button to the edit keybinding dialog #5603

Merged
merged 1 commit into from
Jun 28, 2019

Conversation

vince-fugnitto
Copy link
Member

Description

  • created a custom EditKeybinding dialog used to edit keybindings, and reset custom
    keybindings to their default value. Custom keybindings are keybindings which have
    a User scope (exist in the keymaps.json).
  • the additional functionality added to the edit keybindings dialog is
    used to more easily revert custom keybindings without the need to click
    the revert button in the tree.

Screen Shot 2019-06-27 at 10 00 06 PM

How to Test

  1. open the keybindings-widget: F1 then select Open Keyboard Shortcuts.
  2. update a command's keybinding (using the edit button or double click the row in the table).
  3. click the edit again (once a custom keybinding has been set).
  4. notice the Reset button.
  5. click the Reset button.
  6. the keybinding for the command should be properly reset to it's default value.

Signed-off-by: Vincent Fugnitto [email protected]

@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves keybindings issues related to keybindings labels Jun 28, 2019
@vince-fugnitto vince-fugnitto requested review from lmcbout and fangnx June 28, 2019 02:04
@vince-fugnitto vince-fugnitto self-assigned this Jun 28, 2019
@lmcbout
Copy link
Contributor

lmcbout commented Jun 28, 2019

While testing, I added a filter in the "Search keybindings" field and no "Reset" button was available to return to the default value. As a filter I entered "user" to filter the modified keybindings.

@vince-fugnitto
Copy link
Member Author

vince-fugnitto commented Jun 28, 2019

While testing, I added a filter in the "Search keybindings" field and no "Reset" button was available to return to the default value. As a filter I entered "user" to filter the modified keybindings.

@lmcbout

Do you have any command with a custom keybinding?
Only commands which are edited will display the reset in the table, and the reset in the dialog.

@fangnx
Copy link

fangnx commented Jun 28, 2019

When I tested, it seemed I could still use the old/default keybinding after the edit.

To reproduce this:

  1. Change the keybinding for Add Line Comment from ctrl+k ctrl+x to something else (ctrl+k ctrl+p, for example).
  2. Go to the editor. Both the old and new keybindings should work.

Environment: Chrome + MacOS.


Maybe this is not related to this PR. I tried on master branch and got the same issue.

@lmcbout
Copy link
Contributor

lmcbout commented Jun 28, 2019

While testing, I added a filter in the "Search keybindings" field and no "Reset" button was available to return to the default value. As a filter I entered "user" to filter the modified keybindings.

@lmcbout

Do you have any command with a custom keybinding?
Only commands which are edited will display the reset in the table, and the reset in the dialog.

@vince-fugnitto
I have a few custom keybindings and the reset is not showing

@vince-fugnitto
Copy link
Member Author

While testing, I added a filter in the "Search keybindings" field and no "Reset" button was available to return to the default value. As a filter I entered "user" to filter the modified keybindings.

@lmcbout
Do you have any command with a custom keybinding?
Only commands which are edited will display the reset in the table, and the reset in the dialog.

@vince-fugnitto
I have a few custom keybindings and the reset is not showing

Could you show me? A gif perhaps.

@lmcbout
Copy link
Contributor

lmcbout commented Jun 28, 2019

Also, the reset on the keybinding shortcut page disseapear when entering a filter. See the video, The "Add Folder to workspace" has the rest option until I add a filter, then it only dislay the"Edit"icon"

KeyBindingReset

@vince-fugnitto
Copy link
Member Author

@lmcbout thank you Jacques, the reset button and icon are now present if the search matches the scope column. You can kindly re-test whenever you get the chance.

Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment
Also, when you reset the keybinding, it would be nice if the focus remains on the selected items. Could be done in a separate PR

LGTM .

- created a custom `EditKeybinding` dialog used to edit keybindings, and reset custom
keybindings to their default value. Custom keybindings are keybindings which have
a `User` scope (exist in the `keymaps.json`).
- the additional functionality added to the edit keybindings dialog is
used to more easily revert custom keybindings without the need to click
the revert button in the tree.
- fixed issue where if the `scope` had a fuzzy match and it had `user` scope,
the `reset` icon and button were not preset.

Signed-off-by: Vincent Fugnitto <[email protected]>
@vince-fugnitto
Copy link
Member Author

Thanks guys! @lmcbout @fangnx

@vince-fugnitto vince-fugnitto merged commit 6cf540f into master Jun 28, 2019
@vince-fugnitto vince-fugnitto deleted the vf/keybinding-revert branch June 28, 2019 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement issues that are enhancements to current functionality - nice to haves keybindings issues related to keybindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants