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

feat: add shortcut modifier side #2993

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

Conversation

decodism
Copy link
Contributor

My attempt at #2988.

@decodism decodism force-pushed the master-2 branch 3 times, most recently from 8a92433 to 8e36156 Compare November 18, 2023 18:38
@LiamKearn
Copy link

@decodism Thank you, I've tested locally and this is working fabulously for my use case of using rCMD+Tab for the browser.

@lwouis
Copy link
Owner

lwouis commented Nov 21, 2023

Hi @decodism,

Thank you for sharing this work!

I tested this locally, and it seems to work correctly. I also works for multiple modifiers: all of them need to be on the specified side, for the shortcut to register.

I have an issue with this though: the UX. Merging this PR would mean adding yet another dropdown menu to the preferences UI. It's already very complex and rich, and I'd like to actually decrease the number of elements shown (see #351).

Furthermore, the position of this dropdown is not very clear I think. It says modifier when there could be multiple modifiers, and it's a bit far away from the left input-box, so it could be confusing for people who set for example alt+shift with alt on the left input-box, and shift on the right input-box. They may be confused as to which modifier is referenced by the label.

Overall, i think this feature is a bit too niche to be made mainstream. I'm not sure if there is a way to have a better UX that wouldn't confuse the 99.9% of users who don't need and don't know that both sides of the keyboard can even be distinguished.

What do you think?

Thank you 🙇

@decodism
Copy link
Contributor Author

I've made the UI more compact and made the behavior more useful by allowing another AltTab shortcut to be assigned to the unused side. I'm not so sure about the logic in case of conflict, though.

@LiamKearn
Copy link

@decodism The latest changes seem to break things for me.

When using a modifier side (L or R) windows don't focus on shortcut release (they do if I manually click them). Using "" they work once-more.

@decodism
Copy link
Contributor Author

It's fixed. I completely missed it. Probably needs more testing.

@LiamKearn
Copy link

LiamKearn commented Feb 14, 2024

@lwouis RE: the UI issues, I'm guessing you're opposed to merging "defaults only" features on the grounds that it may add complexity while not getting used by enough users?

e.g. I mean features that are only exposed by documenting that you could do "defaults write etc. etc." to turn on/off xyz.

@lwouis
Copy link
Owner

lwouis commented Feb 14, 2024

Hi @LiamKearn,
Yes indeed. I'd like to avoid adding features that are only used by a few people.
Thank you

@lwouis lwouis force-pushed the master branch 2 times, most recently from 7d7d9cf to 8abb9b4 Compare October 10, 2024 09:28
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.

3 participants