Skip to content

Longest key sequence is not respected #197

@valleybay

Description

@valleybay

Issue: When having a binding such as 'o c' and 'c', typing 'o c' will trigger both 'o c' and 'c'.

I have used Gemini 2.5 Pro to come up with a solution for this problem:

If I were to implement this conflict resolution, my goal would be to ensure that for a single keyboard event, only the most specific (typically the longest) matching keybinding sequence triggers its callback. This prevents scenarios like 'o c' triggering its action, and then the standalone 'c' also triggering its action from the same 'c' key press.

Core Idea:
Instead of executing a callback immediately upon finding a match, collect all keybindings that are completed by the current KeyboardEvent. Then, from this collection of completed bindings, select the "best" one (e.g., the longest sequence) and execute only its callback.

Detailed Steps for Modifying createKeybindingsHandler:

  1. Store Original Key String or Sequence Length: When initially processing the keyBindingMap, alongside the parsed sequence and the callback, also store the original keybinding string or, more directly, the length of the parsed sequence (i.e., the number of presses). This will be used for prioritization.

    let keyBindings = Object.keys(keyBindingMap).map(key => {
        const parsedSequence = parseKeybinding(key);
        return {
            parsedSequence: parsedSequence,
            callback: keyBindingMap[key],
            originalKeyString: key, // Or sequenceLength: parsedSequence.length
            sequenceLength: parsedSequence.length // More direct for prioritization
        } as const;
    });
  2. Collect Completed Matches: Inside the main event listener function, when a key press completes a sequence (i.e., matches is true and remainingExpectedPresses.length would become 0 after processing the current press), don't call callback(event) immediately. Instead, add the details of this completed binding (including its callback, originalKeyString, and sequenceLength) to a temporary list, say completedBindingsForThisEvent.

    // Inside the return event => { ... }
    let completedBindingsForThisEvent: Array<{ callback: (event: KeyboardEvent) => void, sequenceLength: number, originalKeyString: string }> = [];
    
    keyBindings.forEach(keyBindingDetail => {
        let sequence = keyBindingDetail.parsedSequence;
        let callback = keyBindingDetail.callback;
        let sequenceLength = keyBindingDetail.sequenceLength;
        let originalKeyString = keyBindingDetail.originalKeyString;
    
        // ... (existing logic for prev, remainingExpectedPresses, currentExpectedPress, matches)
    
        if (matches) {
            if (remainingExpectedPresses.length > 1) {
                possibleMatches.set(sequence, remainingExpectedPresses.slice(1));
            } else { // This key press completes the sequence
                possibleMatches.delete(sequence);
                completedBindingsForThisEvent.push({ callback, sequenceLength, originalKeyString });
            }
        } else {
            // ... (existing logic for no match)
        }
    });
  3. Prioritize and Execute a Single Callback: After iterating through all keyBindings (i.e., after the forEach loop):

    • If completedBindingsForThisEvent is empty, no binding was completed, so do nothing.
    • If completedBindingsForThisEvent has one entry, execute its callback.
    • If completedBindingsForThisEvent has multiple entries (this is our conflict scenario, e.g., 'o c' and 'c' both match the 'c' key press), iterate through this list and select the one with the maximum sequenceLength. If there are ties for the maximum length (unlikely for the sequence vs. single key problem, but good to consider), you might pick the first one encountered or log a warning about ambiguous bindings.
    • Execute the callback of only this highest-priority (longest sequence) binding.
    // After the keyBindings.forEach loop:
    if (completedBindingsForThisEvent.length > 0) {
        let bestMatch = completedBindingsForThisEvent.reduce((currentBest, candidate) => {
            if (!currentBest || candidate.sequenceLength > currentBest.sequenceLength) {
                return candidate;
            }
            // Optional: Handle ties in sequenceLength. For now, first one wins in a tie.
            // Or, if originalKeyString is more descriptive for tie-breaking (e.g. for identical lengths)
            return currentBest;
        });
    
        if (bestMatch) {
            bestMatch.callback(event);
        }
    }
  4. Timeout Logic: The existing timeout logic for clearing possibleMatches would remain the same, as it handles incomplete sequences.

Why this approach would work:

  • Explicit Prioritization: It explicitly chooses which callback to fire based on a defined rule (longest sequence takes precedence).
  • Single Invocation: It ensures only one callback is invoked per KeyboardEvent that completes one or more bindings, resolving the "double trigger."
  • Minimal Disruption: It modifies the core matching loop minimally, adding a collection step and a post-loop decision step.

Potential Configuration:
This "longest match wins" behavior could be made the default. Optionally, an option could be added to KeyBindingHandlerOptions to control this prioritization, e.g., multiMatchStrategy: "longest" | "all" | "first" (where "all" would be the current behavior).

This modification would make tinykeys more robust in handling these types of overlapping definitions directly within the library.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions