-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(combos): clear combo timer only when affected keys are released #2494
base: main
Are you sure you want to change the base?
Conversation
1fdd5d6
to
5a26255
Compare
I added one more failing test and a fix for an annoying issue where, if a combo overlaps (so it can't be resolved immediately) and is followed by another combo, only the first one is resolved. I think thats what actually happens in #1899 (since taipo using 3-key overlapping combos of course). For me it was impossible to actuate both parens in a row (on middle+index opening at left hand, closing at right hand) due to overlapping combo with higher term (on ring+middle+index) on both hands. The fix is to not release last keypress but reraise it (if there is fully pressed combo) so it can be used by the second combo. |
2e65f37
to
11c58a8
Compare
Added a third test and a fix for the same issue as the original one, but for active combo release. This PR now addresses the following issues:
|
Test the following conditions: - Combo is not interrupted if any other key (even without combos assigned to it) is released (tests/other-key-release). - Combo is not interrupted if any key from an active combo is released (tests/other-key-release-2). - A combo with an overlapping combo on the same spot (making it unable to resolve immediately) is not interrupted if another combo candidate is initialized, preventing quick combo rolls (tests/overlapping-combos-5).
11c58a8
to
ecbaaaf
Compare
I wonder if we might run into issues with the inverse problem. Say you hypothetically have |
Which condition do you mean - the first one, I guess? I would expect it to trigger if the conditions are met (timeout and prior idle), and I don’t see how shift should be involved here (but I see how both behaviors could be desired). But I see what you mean. I’ve been using it for the last 4 months without any unexpected actuations, but I don’t use the real (held) shift – I use sticky shift instead. So, maybe it’s worth waiting for someone who uses a real shift to give it a try? In the worst case, this behavior could be hidden under some flag. |
Now, the key release listener checks if the released key is part of any combo candidate or active combo, instead of stopping the combo timer on any key release.
I also had to modify the
tap-dance/3b-tap-int-seq
test because it relied on the old combo release behavior, but I would like to double-check if this is the behavior we want to maintain.Fixes #2378