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.

Sign up for GitHub

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

Jump to bottom

Make common hotkeys visible as Hotkeys #9898

Merged
merged 3 commits into from Mar 25, 2022
Merged

Make common hotkeys visible as Hotkeys #9898

merged 3 commits into from Mar 25, 2022

Conversation

mstv
Copy link
Member

@mstv mstv commented Mar 14, 2022 •
edited

Fixes last part of #9861
and #9723 (comment)

Proposed changes

  • Replace hotkeys hardcoded in various KeyUp / KeyDown handlers with Hotkeys for
    • FormCommit
    • Left Panel
    • RevisionGridControl
  • Left Panel: Translate CTRL+[SHIFT]+Space into node click in order to support upcoming multi-selection

Screenshots

After

image

image

image

Test methodology

  • manual

Test environment(s)

  • Git Extensions 33.33.33
  • Build e4920d9
  • Git 2.35.1.windows.1
  • Microsoft Windows NT 10.0.19044.0
  • .NET 5.0.12
  • DPI 96dpi (no scaling)

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).

✒️ I contribute this code under The Developer Certificate of Origin.

@mstv mstv self-assigned this Mar 14, 2022
gerhardol
gerhardol reviewed Mar 15, 2022
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

have not run

GitUI/Hotkey/HotkeySettingsManager.cs Outdated
Hk(RepoObjectsTree.Command.Rename, Keys.F2),
Hk(RepoObjectsTree.Command.Search, Keys.F3),
Hk(RepoObjectsTree.Command.MultiSelect, Keys.Control | Keys.Space),
Hk(RepoObjectsTree.Command.MultiSelectWithChildren, Keys.Control | Keys.Alt | Keys.Space)),
Copy link
Member

Choose a reason for hiding this comment

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

Ctrl-Shift instead?
(Is this OK with Alt-Click to select remote/local tracked?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ctrl-Shift instead?

Valid point! So, #9723 should switch to CTRL+SHIFT+Click, too, @h0lg.
(It is even more related to selecting than the combination with ALT. I do not see a conflict with SHIFT+Click.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed and updated the PR description.

@RussKie
Copy link
Member

RussKie commented Mar 16, 2022 via email •
edited by ghost

@h0lg h0lg mentioned this pull request Mar 16, 2022
Merged
RussKie
RussKie reviewed Mar 17, 2022
View reviewed changes
GitUI/CommandsDialogs/FormCommit.cs Outdated
Comment on lines 676 to 681
SelectNext = 19,
SelectNext2 = 20,
SelectNext3 = 21,
SelectPrevious = 22,
SelectPrevious2 = 23,
SelectPrevious3 = 24,
Copy link
Member

Choose a reason for hiding this comment

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

Can you please annotate what these are?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in abe3612 if I got you right.

Copy link
Member

Choose a reason for hiding this comment

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

My guess was that why there are SelectNext , SelectNext2, SelectNext3
There can only be one hotkey for each command and here there are three key combinations for the same thing.
Maybe add a comment about that regardless

Copy link
Member Author

Choose a reason for hiding this comment

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

Or rather

   SelectNext = 19,
   SelectNext_AlternativeHotkey1 = 20,
   SelectNext_AlternativeHotkey2 = 21,

because these names are visible in the HotkeySettingsPage.

gerhardol
gerhardol reviewed Mar 19, 2022
View reviewed changes
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

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

One shortcut change that could be seen as a regression.
Not blocking merge.

GitUI/Hotkey/HotkeySettingsManager.cs
Hk(RepoObjectsTree.Command.Delete, Keys.Delete),
Hk(RepoObjectsTree.Command.Rename, Keys.F2),
Hk(RepoObjectsTree.Command.Search, Keys.F3),
Hk(RepoObjectsTree.Command.MultiSelect, Keys.Control | Keys.Space),
Copy link
Member

Choose a reason for hiding this comment

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

This overrides the current (global)shortcut, opening FormCommit
No good suggestion right now...

Copy link
Member Author

Choose a reason for hiding this comment

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

Only the Left Panel is affected. Overriding the hotkey Ctrl+Space there is intentional because it is the standard hotkey for manipulating multi-selections using the keyboard.
#9723 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

It could be seen as a regression for users that do not change focus when submitting
No better suggestion, approving

@mstv mstv requested a review from RussKie March 20, 2022 12:49
RussKie
RussKie reviewed Mar 25, 2022
View reviewed changes
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

:shipit:

@mstv mstv merged commit 965e86c into gitextensions:master Mar 25, 2022
mstv added a commit that referenced this pull request Mar 25, 2022
@mstv
RevisionGridControl: Replace KeyUp/Down with Hotkeys (#9898)
82fd53e
mstv added a commit that referenced this pull request Mar 25, 2022
@mstv
ROT: Replace PreviewKeyDown with Hotkeys (#9898)
a504409
@ghost ghost added this to the vNext milestone Mar 25, 2022
@mstv mstv deleted the feature/9861_hotkeys branch March 25, 2022 18:35
@RussKie RussKie mentioned this pull request Mar 29, 2022
Closed
@mstv mstv mentioned this pull request Apr 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussKie RussKie RussKie left review comments

@gerhardol gerhardol gerhardol approved these changes

Assignees

@mstv mstv

Labels
None yet
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mstv @RussKie @gerhardol