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

Keyboard handler rewrite #332

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Spengreb
Copy link
Contributor

@Spengreb Spengreb commented Jun 24, 2024

I rewrote how LUPPP handles the keyboard shortcuts. This pull request adds a new context menu button when right clicking on a clip. It shows the current hotkey assigned to this clip and allows the user to change it with a single button press. If another hotkey is already assigned somewhere else it will remove that assignment applying it to only the current clip in context.

Heres what it looks like:

image

This addresses quite a few problems from the previous implementation. One thing was that the hotkeys are not communicated in any way to the user and when first using LUPPP it was a little confusing, especially when not using a US/UK qwerty keyboard.

Hotkeys are saved to the luppp.prfs file and it now auto saves the prfs on exit.

As it stands in this PR the new hotkeys are only set for that instance of the application. I want this to persist from one luppp launch to the other and I would like to make the other special keys configurable too in the setup menu. To prevent this branch from ballooning too much I think I'm at a point where its good enough to merge.

Copy link
Member

@harryhaaren harryhaaren left a comment

Choose a reason for hiding this comment

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

try {} catch {} or atoi for avoiding C++ exception throwing. Elegant solution (much more so than my "i'm learning C++ so duplicating code everywhere" approach :)

src/diskreader.cxx Outdated Show resolved Hide resolved
src/diskreader.cxx Outdated Show resolved Hide resolved
@Spengreb
Copy link
Contributor Author

The above comments should all be resolved. I also did a bit of a refactor of the hotkey dialog box as it was a bit messy before. I fixed some bugs i noticed along the way also. Such as not allowing SHIFT and ESC to be set as a hotkey. Since SHIFT is the inverse of an action, it caused some very confusing behavior if allowed to be set as a hotkey as well.

I also changed the format of hotkey dialog on the front end to try to communicate some of this to the user here is what it looks like now:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants