Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export class PressedKeys {
has(...keys: string[]): boolean {
this.#subscribe?.();
const normalizedKeys = keys.map((key) => key.toLowerCase());
return normalizedKeys.every((key) => this.#pressedKeys.includes(key));
return normalizedKeys.length === this.#pressedKeys.length &&
normalizedKeys.every((key) => this.#pressedKeys.includes(key));
}

get all(): string[] {
Expand Down
188 changes: 188 additions & 0 deletions packages/runed/src/lib/utilities/pressed-keys/pressed-keys.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
import { describe, expect, it, vi } from "vitest";
import { PressedKeys } from "./pressed-keys.svelte.js";
import { testWithEffect } from "$lib/internal/test-utils.svelte.js";

describe("PressedKeys", () => {
describe("has method", () => {
it("should use exact matching by default", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

// Press only 'k'
target.dispatchEvent(new KeyboardEvent("keydown", { key: "k" }));
expect(keys.has("k")).toBe(true);
expect(keys.has("K")).toBe(true); // case insensitive

// Now press 'meta' while 'k' is still down
target.dispatchEvent(new KeyboardEvent("keydown", { key: "Meta" }));
expect(keys.has("k")).toBe(false); // exact match fails
expect(keys.has("meta", "k")).toBe(true); // exact match for both keys
expect(keys.has("k", "meta")).toBe(true); // order doesn't matter

// Release 'meta'
target.dispatchEvent(new KeyboardEvent("keyup", { key: "Meta" }));
expect(keys.has("k")).toBe(true); // back to just 'k'

// Release 'k'
target.dispatchEvent(new KeyboardEvent("keyup", { key: "k" }));
expect(keys.has("k")).toBe(false);
});
});


it("should handle modifier keys correctly", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

// Test each modifier
const modifiers = ["Control", "Alt", "Shift", "Meta"];

for (const modifier of modifiers) {
target.dispatchEvent(new KeyboardEvent("keydown", { key: modifier }));
expect(keys.has(modifier.toLowerCase())).toBe(true);
target.dispatchEvent(new KeyboardEvent("keyup", { key: modifier }));
}
});
});

it("should clear keys on blur", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

target.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
target.dispatchEvent(new KeyboardEvent("keydown", { key: "b" }));
expect(keys.has("a", "b")).toBe(true);

window.dispatchEvent(new Event("blur"));
expect(keys.all).toEqual([]);
expect(keys.has("a")).toBe(false);
expect(keys.has("b")).toBe(false);
});
});

it("should clear keys on visibility change", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

target.dispatchEvent(new KeyboardEvent("keydown", { key: "x" }));
expect(keys.has("x")).toBe(true);

// Mock hidden visibility
Object.defineProperty(document, "visibilityState", {
value: "hidden",
writable: true,
});
document.dispatchEvent(new Event("visibilitychange"));

expect(keys.all).toEqual([]);
expect(keys.has("x")).toBe(false);
});
});
});

describe("onKeys method", () => {
it("should use exact matching by default", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;
const callback = vi.fn();

keys.onKeys("k", callback);

// Press only 'k' - should trigger
target.dispatchEvent(new KeyboardEvent("keydown", { key: "k" }));
expect(callback).toHaveBeenCalledTimes(1);

// Press 'meta' while 'k' is down - should not trigger again
target.dispatchEvent(new KeyboardEvent("keydown", { key: "Meta" }));
expect(callback).toHaveBeenCalledTimes(1); // still 1

// Release 'meta' - should trigger again as we're back to just 'k'
target.dispatchEvent(new KeyboardEvent("keyup", { key: "Meta" }));
expect(callback).toHaveBeenCalledTimes(2);
});
});


it("should handle multiple keys with exact matching", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;
const callback = vi.fn();

keys.onKeys(["meta", "k"], callback);

// Press only 'k' - should not trigger
target.dispatchEvent(new KeyboardEvent("keydown", { key: "k" }));
expect(callback).toHaveBeenCalledTimes(0);

// Press 'meta' - should trigger
target.dispatchEvent(new KeyboardEvent("keydown", { key: "Meta" }));
expect(callback).toHaveBeenCalledTimes(1);

// Press 'shift' - should not trigger again (exact match fails)
target.dispatchEvent(new KeyboardEvent("keydown", { key: "Shift" }));
expect(callback).toHaveBeenCalledTimes(1);
});
});
});

describe("all property", () => {
it("should return all pressed keys", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

expect(keys.all).toEqual([]);

target.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
expect(keys.all).toEqual(["a"]);

target.dispatchEvent(new KeyboardEvent("keydown", { key: "B" }));
expect(keys.all).toEqual(["a", "b"]);

target.dispatchEvent(new KeyboardEvent("keyup", { key: "a" }));
expect(keys.all).toEqual(["b"]);
});
});
});

describe("edge cases", () => {
it("should handle repeated keydown events for same key", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

target.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
target.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
target.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));

expect(keys.all).toEqual(["a"]);
expect(keys.has("a")).toBe(true);
});
});

it("should handle modifier release clearing non-modifier keys", async () => {
await testWithEffect(async () => {
const keys = new PressedKeys();
const target = document.body;

// Press several keys including modifiers
target.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
target.dispatchEvent(new KeyboardEvent("keydown", { key: "Meta" }));
target.dispatchEvent(new KeyboardEvent("keydown", { key: "b" }));
target.dispatchEvent(new KeyboardEvent("keydown", { key: "Shift" }));

expect(keys.all.sort()).toEqual(["a", "b", "meta", "shift"]);

// Release one modifier - should clear non-modifiers
target.dispatchEvent(new KeyboardEvent("keyup", { key: "Meta" }));
expect(keys.all).toEqual(["shift"]);
});
});
});
});
36 changes: 34 additions & 2 deletions sites/docs/src/content/utilities/pressed-keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,50 @@ const isArrowDownPressed = $derived(keys.has("ArrowDown"));
const isCtrlAPressed = $derived(keys.has("Control", "a"));
```

Or get all of the currently pressed keys:
`has` uses **exact matching** - it only returns `true` when exactly the specified keys are pressed:

```ts
const keys = new PressedKeys();

// If only 'k' is pressed:
keys.has("k"); // true
keys.has("k", "meta"); // false

// If 'meta' + 'k' are pressed:
keys.has("k"); // false (exact match requires only 'k')
keys.has("k", "meta"); // true
```

If you need to check whether a specific key is pressed regardless of modifiers, use the `all` property:

```ts
// If 'meta' + 'k' are pressed:
keys.all.includes("k"); // true
keys.all.includes("meta"); // true
```

### Getting All Pressed Keys

Get all of the currently pressed keys:

```ts
const keys = new PressedKeys();
console.log(keys.all);
```

Or register a callback to execute when specified key combination is pressed:
### Monitoring Key Combinations

Register a callback to execute when specified key combination is pressed:

```ts
const keys = new PressedKeys();
// Triggers only when exactly meta+k are pressed
keys.onKeys(["meta", "k"], () => {
console.log("open command palette");
});

// Triggers only when 'k' is pressed alone
keys.onKeys("k", () => {
console.log("k pressed without modifiers");
});
```
74 changes: 74 additions & 0 deletions spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# PressedKeys Ergonomics Improvement Plan

## Problem Statement
The current `PressedKeys` implementation has an ergonomics issue where checking for a single key (e.g., `has("k")`) also matches when modifiers are pressed (e.g., Cmd+K). This doesn't align with user expectations or industry best practices.

## Todo List
- [x] Update the `has` method to use exact matching only
- [x] Update the `onKeys` method to use exact matching only
- [x] Remove inclusive option entirely for simpler API
- [x] Add comprehensive tests for exact matching behavior
- [x] Update documentation with examples of exact matching
- [x] Write PR description explaining the breaking change

## Implementation Details

### Option 1: Add options parameter to existing methods
```typescript
// Default behavior (backward compatible - inclusive match)
keys.has("k") // true for both "k" and "Cmd+k"

// New exact match option
keys.has("k", { exact: true }) // true only for "k" alone
keys.has(["meta", "k"], { exact: true }) // true only for exactly Cmd+k

// For onKeys
keys.onKeys("k", callback, { exact: true }) // fires only for "k" alone
```

### Option 2: Add new methods for exact matching
```typescript
// Existing methods remain unchanged (inclusive)
keys.has("k") // true for both "k" and "Cmd+k"

// New exact methods
keys.hasExact("k") // true only for "k" alone
keys.onKeysExact("k", callback) // fires only for "k" alone
```

### Option 3: Change default behavior (breaking change)
Make exact matching the default and add an option for inclusive matching. This would be more aligned with user expectations but would be a breaking change.

## Recommendation
I recommend **Option 1** as it:
- Maintains backward compatibility
- Provides clear, explicit control over matching behavior
- Aligns with patterns used in other parts of the codebase
- Makes the API more flexible for different use cases

## Test Cases to Add
1. `has("k")` returns true when only "k" is pressed
2. `has("k")` returns true when "Cmd+k" is pressed (backward compatibility)
3. `has("k", { exact: true })` returns true only when "k" is pressed alone
4. `has("k", { exact: true })` returns false when "Cmd+k" is pressed
5. `has(["meta", "k"], { exact: true })` returns true only for exactly Cmd+k
6. `onKeys` with exact option fires appropriately
7. Edge cases with multiple modifiers

## Review

### Summary of Changes
1. **Simplified the API** - Removed the `KeyMatchOptions` type and `inclusive` option entirely
2. **Made exact matching the only behavior** - `has()` and `onKeys()` now only match when exactly the specified keys are pressed
3. **Updated tests** - Removed tests for inclusive matching and kept only exact matching tests
4. **Breaking change** - Users who relied on the previous inclusive behavior will need to update their code

### Key Decisions
- Chose simplicity over backward compatibility by removing the inclusive option
- This makes the API more predictable and aligns with user expectations
- Users who need inclusive behavior can use `keys.all.includes("k")` directly

### Migration Impact
- Any code using `has("k")` that expected it to match "Cmd+K" will break
- The migration path is clear: either specify all keys or use `keys.all.includes()`
- Documentation has been updated to explain the new behavior
Loading