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

Changed the way Paste keybind works in Composer #488

Merged
merged 6 commits into from
Sep 21, 2023

Conversation

gaussandhisgun
Copy link
Contributor

As it is, the Paste keybind in Composer's attachment page relies on Gdk.Key.* and keyvals. But as it turns out, that makes alphabetical keys based keybinds not work at all on non-latin layouts (which i am a user of), or shift around on layouts that aren't default QWERTY. And while that makes sense for, say, Dvorak, for Cyrillic it makes the users like me switch to a different keyboard layout just to paste an image. Which, again, is against Gnome's principles. So, I did the only right thing that came to my mind, and added an if that also checks for keycode - so pressing Ctrl+cyrillic М or Ctrl+whatever-is-in-the-place-of-V will also paste the image. Keeping the keyval approach because of all those poor Dvorak people who think about what keys they press and not where they are located.

P. S. This is my first pull request. No, really, I've just figured out how to git push an hour ago. Probably would be a good idea to do something like this for other keybinds?

@GeopJr
Copy link
Owner

GeopJr commented Sep 4, 2023

Thanks for making this PR!

I don't really like using the physical key's location as a trigger. While it makes sense for QWERTY, other layout users (as you and the linked thread mentioned) don't mentally map shortcuts to their QWERTY locations but rather create new shortcuts. Additionally, there's no way to write docs for it (since it's position-based).

Having both the keyval and the keycode as triggers is not appropriate either. When their physical locations do not match (not QWERTY) there will be 2 different shortcuts that do the exact same thing, one of which might be already taken.

Would you mind if I postpone this for a bit so I investigate some more? I'm leaning towards making it an action and letting GTK deal with it or using the key translation functions (Gdk.Display#translate_key, Gdk.Keymap#translate_keyboard_state).

[off-topic]

  1. Great work on your first PR!
  2. You don't have to defend bug fixes 😅, that HIG section is about workflows, not... bugs

[/off-topic]

Verified

This commit was signed with the committer’s verified signature.
GeopJr Evan Paterakis
…s better for UX or something."

This reverts commit cf029aa.

Verified

This commit was signed with the committer’s verified signature.
GeopJr Evan Paterakis
…nts tab, from keyvalue to keycode"

This reverts commit c0c7ba3.

Verified

This commit was signed with the committer’s verified signature.
GeopJr Evan Paterakis
@GeopJr GeopJr merged commit f983735 into GeopJr:main Sep 21, 2023
@GeopJr
Copy link
Owner

GeopJr commented Sep 21, 2023

Ended up using an action, thanks for the info and help!

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.

None yet

2 participants