- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 63
feat: use exact matching for PressedKeys #280
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
base: main
Are you sure you want to change the base?
feat: use exact matching for PressedKeys #280
Conversation
| 
 | 
| I think this makes a lot of sense. @TGlide since this is your brain child, what do you think? | 
| built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
 | 
| Sorry for the huge wait! let me check | 
| I propose a different solution. A mix of 1 and something else. const keys = new PressedKeys({ exact: true })
// When only "k" is pressed:
keys.has("k") // true
keys.has("k", "meta") // false
// When Cmd+K is pressed:
keys.has("k") // false (exact match)
keys.has("k", "meta") // true (exact match)
// Or... Old behaviour
const keys = new PressedKeys()
// When only "k" is pressed:
keys.has("k") // true
keys.has("k", "meta") // false
// When Cmd+K is pressed:
keys.has("k") // true
keys.has("k", { exact: true }) // true
keys.has("k", "meta") // true (exact match)I do find it more intuitive that keys.has("k") represents that k is pressed no matter what. But I can see the value for exact matching! | 
| For me a version of Option 2 makes the most sense. Having two functions makes them equally accessible. const keys = new PressedKeys()
if (keys.matchOne("ArrowDown", "j")) {
  // Cursor down
}
if (keys.matchOne("ArrowUp", "k")) {
  // Cursor up
}
if (keys.match("Control", "z")) {
  // Undo
}
if (keys.has("w") {
  let speedZ = 1;
  if (keys.has("Shift") {
    // sneak
    speedZ = 0.5;
  }
  // move forward
}Maybe the version with  // Edit: improved examples | 
| I wouldn't mind option 2 either, tbh! | 
| 100% agree the current behavior is counterintuitive. However, I'd ALSO like to propose a variation of Option 2 that I think gives us the best of all worlds: keys.hasAny("k") // true if k is pressed (with or without modifiers)
keys.hasExact("k") // true only if k alone is pressed
keys.hasExact("meta", "z") // true only if cmd+z is pressed
keys.hasOne("j", "ArrowDown") // true if ANY of these keys are pressedI feel comfortable breaking  When someone reads  Explicit > implicit in this case. | 
| @huntabyte +1, really like the explicitness of hasAny and hasExact! | 
| Now that I think more about it, we'd probably also need like  | 
| Wouldn't that be covered by  Also, was wondering if we could consider the semantics being  | 
| 
 Agreed! 
 Not sure to be honest. As @braden-w said, a  
 Agreed! | 
This PR changes the default behavior of the
PressedKeysutility to use exact key matching instead of inclusive matching. This meanshas("k")will now only returntruewhen ONLY the "k" key is pressed, not when "Cmd+K" is pressed.Problem
The previous behavior was counterintuitive and didn't align with user expectations or industry best practices:
This made it impossible to distinguish between a single key press and a key combination, leading to unexpected behavior in applications.
Solution Options Considered
Option 1: Add options parameter (backward compatible)
Option 2: Add new methods
Option 3: Change default behavior (breaking change) ✅
Make exact matching the default and add an option for inclusive matching.
Implementation (Option 3 - Simplified)
I implemented Option 3 with a simplified approach - exact matching is now the only behavior:
Changes Made
hasmethod to use exact matching onlyonKeysmethod to use exact matching onlyBreaking Changes
has("k")will no longer match when modifier keys are also pressedonKeys("k", callback)will no longer fire when modifier keys are also pressedMigration Guide
The previous inclusive behavior can be achieved using
keys.all.includes():This gives users a clear migration path - they can simply replace
keys.has("k")withkeys.all.includes("k")to maintain the old behavior.Benefits
Discussion
While I implemented Option 3 due to its superior ergonomics, this decision is open for discussion. The final implementation removes the
inclusiveoption entirely to maintain a simpler, more focused API.If users need to check whether a key is pressed regardless of modifiers, they can use
keys.all.includes("k")directly.I believe the benefits of the more intuitive API outweigh the breaking change, and the simplified implementation (without the inclusive option) makes the behavior clear and predictable.