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

Add handling for INS, DEL, NUM LOCK, and some shifted characters. #2553

Merged
merged 4 commits into from
May 6, 2024

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented May 5, 2024

Adds some missing key definitions for INSERT, DELETE and NUMLOCK, and some other esoteric keys; and improves shift handling for GTK.

Fixes #2220.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Member

@rmartin16 rmartin16 left a comment

Choose a reason for hiding this comment

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

These work; although, do you want to handle CTRL, ALT, and SUPER similarly to shift? I still see those warnings.

Also, when I tab to the Calculate button in Tutotial1 example, I get a system beep. Is that expected?

@freakboy3742
Copy link
Member Author

These work; although, do you want to handle CTRL, ALT, and SUPER similarly to shift? I still see those warnings.

I've just added handling for these.

Also, when I tab to the Calculate button in Tutotial1 example, I get a system beep. Is that expected?

... it is not. Bother. It was being masked in my testing because my VM didn't have its "speaker" attached.

@freakboy3742
Copy link
Member Author

The beep appears to be pre-existing... I've opened #2554 to track it.

@freakboy3742 freakboy3742 requested a review from rmartin16 May 6, 2024 01:42
Copy link
Member

@rmartin16 rmartin16 left a comment

Choose a reason for hiding this comment

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

One more interesting one...if I hold CTRL or ALT and then press SUPER, I can still get the warning for SUPER...not necessarily a common combo but it is used by window placement mgmt.

Also, just as note, noticing that the ALT+TAB icon is using the system default on Ubuntu and Fedora....may just be a Gnome thing though.

@freakboy3742
Copy link
Member Author

The CTRL+ALT+WIN key combo appears to be coming through as a completely different key, rather than a combination of the three individual keys - but you can bind to MOD_1+MOD_2+MOD_3+a.

Based on the GTK docs, all this handling is a bit abstract anyway, because it can be redefined at the X11 level - I'm guessing that is what is happening here.

Rather than try to chase our tail here, I've opted to add a generic "If unknown, be quiet" handler; this only impacts on the key handling in TextInput, so I think we're OK to ignore weird combos.

Copy link
Member

@rmartin16 rmartin16 left a comment

Choose a reason for hiding this comment

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

This tradeoff makes sense. fwiw, looks like Android also just ignores undefined keys.

@freakboy3742 freakboy3742 merged commit b546ab7 into beeware:main May 6, 2024
34 checks passed
@freakboy3742 freakboy3742 deleted the gtk-keys branch May 6, 2024 05:48
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.

Missing GDK key definitions
2 participants