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

Detect Shortcut: Hold Esc/Enter to Cancel/Accept #2135

Merged
merged 6 commits into from
Apr 16, 2020
Merged

Detect Shortcut: Hold Esc/Enter to Cancel/Accept #2135

merged 6 commits into from
Apr 16, 2020

Conversation

traies
Copy link
Member

@traies traies commented Apr 15, 2020 •
edited
Loading

Summary of the Pull Request

Apply/Discard changes on the Detect SingleKey/Shortcut ContentDialog by holding down Enter/Escape.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • When the detect single key/shortcut content dialog is created, two KeyDelays objects are registered to track the Escape and Enter keys.
  • For each KeyDelay object, three callbacks are provided, one for Short Press, and two for Long Press (when pressed initially and when released).
  • Long Press triggers when the user holds the key for more that 0.9 seconds.
  • When the callback function needs to close the window, the UnregisterKeyDelay code should be run from a different thread, otherwise there would be a deadlock.
  • The Detect shortcut content dialog takes into account single key remapping when tracking Escape and Enter.

Validation Steps Performed

  • Holding down the Escape key has the same effect as clicking on Cancel on the content dialog.
  • Holding down the Enter key has the same effect as clicking (and holding) on OK on the content dialog.
  • Pressing and releasing early either the Enter or Escape key detects the key value.

@traies traies force-pushed the dev/traies/HoldKeyToQuit branch 2 times, most recently from e9e3a3b to 2b0abf9 Compare April 15, 2020 15:53
@crutkas crutkas added the Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager label Apr 15, 2020
@traies traies requested review from arjunbalgovind and a team April 15, 2020 20:18
@traies traies marked this pull request as ready for review April 15, 2020 20:18
@arjunbalgovind
Copy link
Contributor

arjunbalgovind commented Apr 15, 2020 •
edited
Loading

It looks like your commits aren't linked to your GitHub ID. I think you have to set your email ID in the git config on your machine like this
git config --global user.email [email protected]

arjunbalgovind
arjunbalgovind reviewed Apr 15, 2020
View reviewed changes
PowerToys.sln Outdated Show resolved Hide resolved
src/modules/keyboardmanager/common/KeyDelay.h Outdated Show resolved Hide resolved
src/modules/keyboardmanager/common/KeyboardManagerState.cpp Outdated Show resolved Hide resolved
src/modules/keyboardmanager/common/KeyboardManagerState.cpp Show resolved Hide resolved
src/modules/keyboardmanager/dll/dllmain.cpp Outdated Show resolved Hide resolved
@traies
Detect Shortcut: Hold Esc/Enter to Discard/Apply changes
57279d1 
Bypass shorcut/single key remapping by holding the navigation keys
@traies
KeyDelay commented
84ff97d
@traies
Refactoring and fixes
fbd21e4 
* Fix issue where Esc and Enter would not be removed in Detect Shortcut
* Fix issue where Esc and Enter would behave as delayed keys in the Detect Shortcut
dialog after being mapped.
* Refactored duplicated code in ShortcutControl and SingleKeyRemapControl.
*
@traies
Fix code style
da25d6c
arjunbalgovind
arjunbalgovind reviewed Apr 16, 2020
View reviewed changes
src/modules/keyboardmanager/common/KeyDelay.cpp
}
}

if (CheckIfMillisHaveElapsed(_initialHoldKeyDown, GetTickCount(), LONG_PRESS_DELAY_MILLIS))
Copy link
Contributor

Choose a reason for hiding this comment

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

GetTickCount resets to 0 every 49 days. does that match the behaviour for the ev.time?

arjunbalgovind
arjunbalgovind reviewed Apr 16, 2020
View reviewed changes
src/modules/keyboardmanager/common/KeyboardManagerState.h Outdated
Comment on lines 135 to 149
void RegisterKeyDelay(
DWORD key,
std::function<void(DWORD)> onShortPress,
std::function<void(DWORD)> onLongPressDetected,
std::function<void(DWORD)> onLongPressReleased);

void UnregisterKeyDelay(DWORD key);

bool HandleKeyDelayEvent(LowlevelKeyboardEvent* ev);

void SelectDetectedRemapKey(DWORD key);

void SelectDetectedShortcut(DWORD key);

void ResetDetectedShortcutKey(DWORD key);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the single line comments here as well?

arjunbalgovind
arjunbalgovind approved these changes Apr 16, 2020
View reviewed changes
Copy link
Contributor

@arjunbalgovind arjunbalgovind left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! The logic for handling the delay is really cool!

@traies traies merged commit c37884b into dev/build-features Apr 16, 2020
@traies traies deleted the dev/traies/HoldKeyToQuit branch April 21, 2020 17:59
@Itchibon777 Itchibon777 mentioned this pull request Dec 14, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arjunbalgovind arjunbalgovind arjunbalgovind approved these changes

Assignees
No one assigned
Labels
Product-Keyboard Shortcut Manager Issues regarding Keyboard Shortcut Manager
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@traies @arjunbalgovind @crutkas