feat: keystroke overlay (record keys + built-in "Keystrokes" extension)#694
Open
flol3622 wants to merge 3 commits into
Open
feat: keystroke overlay (record keys + built-in "Keystrokes" extension)#694flol3622 wants to merge 3 commits into
flol3622 wants to merge 3 commits into
Conversation
…ension API Wire up uiohook-napi keyboard events alongside existing mouse capture to record keystrokes with playback timestamps. Save to a .keys.json sidecar on recording stop (mirrors .cursor.json pattern). Expose via IPC + extensionHost.setKeystrokeEvents so getKeystrokesInRange() works in extensions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nimation - Read modifier state from uiohook event booleans (shiftKey/ctrlKey/altKey/ metaKey) instead of hand-tracking keycodes — fixes Shift not showing in chords like Shift+Cmd+V, and deletes the manual modifier set + keyup handler - Correct the extended nav keycodes (Home/End/PageUp/PageDown/Insert/Delete) to match uiohook-napi's UiohookKey constants - Bundle the overlay as a built-in extension (public/builtin-extensions/ keystrokes) so it auto-activates and ships with the app; settings nest under the cursor section via parentSection - keyviz-style entrance/exit animation (pop + slide + fade) and configurable horizontal/vertical margins; drop shadow on badges Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Redesign overlay as white 3D keycaps (extruded dark base, soft shadow, black glyphs) sized to the canvas; modifiers render glyph + label stacked - Add Keyboard layout setting (QWERTY/AZERTY/QWERTZ): uiohook reports physical key positions, so AZERTY 'A' (physical Q slot) was showing as Q. Remap fixes the common non-US layouts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
|
Contributor
📝 WalkthroughWalkthroughThis PR adds end-to-end keystroke telemetry to Recordly. During recording, keyboard events are captured via uiohook-napi, converted to ChangesKeystroke Telemetry & Overlay Extension
Sequence DiagramsequenceDiagram
participant uiohook as uiohook-napi
participant interaction as interaction.ts
participant telemetry as cursor/telemetry.ts
participant recording as recording stop (mac/win)
participant ipcMain as get-keystroke-telemetry
participant VideoEditor as VideoEditor
participant extension as keystrokes/index.js
uiohook->>interaction: keydown (HookKeyboardEvent)
interaction->>telemetry: pushKeystroke({ timeMs, key, modifiers })
recording->>telemetry: saveKeystrokeTelemetry(videoPath)
telemetry-->>recording: writes .keys.json sidecar
recording->>telemetry: resetKeystrokeTelemetry()
VideoEditor->>ipcMain: getKeystrokeTelemetry(videoSourcePath)
ipcMain->>telemetry: loadKeystrokeTelemetry(videoPath)
telemetry-->>ipcMain: KeystrokeEvent[]
ipcMain-->>VideoEditor: { success, events }
VideoEditor->>extension: extensionHost.setKeystrokeEvents(events)
extension->>extension: final render hook draws keycap badges on canvas
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
Contributor
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ipc/cursor/telemetry.ts`:
- Around line 370-377: The filter function that validates KeystrokeEvent objects
is only checking that the modifiers property is an array, but not validating
that each element within the array is a string. This allows malformed telemetry
with non-string modifier values to pass through the IPC boundary. Add an
additional validation check to the filter condition that ensures every element
in the modifiers array is of type string, in addition to the existing
Array.isArray check on line 376.
In `@electron/ipc/register/recording.ts`:
- Around line 1003-1008: The resetKeystrokeTelemetry() call at the native stop
flow is executed before finalizeStoredVideo() is invoked later in the mux flow
(around line 1473), which causes the keystroke buffer to be cleared prematurely.
When finalizeStoredVideo() subsequently attempts to save keystrokes and
encounters an empty buffer, it triggers the delete-on-empty branch and removes
the keystroke sidecar file that was just saved. Move the
resetKeystrokeTelemetry() call to execute after finalizeStoredVideo() completes,
and also add resetKeystrokeTelemetry() calls to all terminal failure exit paths
that do not reach finalizeStoredVideo() to prevent stale keystroke data from
persisting across multiple recordings.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 3341-3353: The useEffect hook fetching keystroke telemetry via
window.electronAPI.getKeystrokeTelemetry() has a race condition where stale
responses from previous videoSourcePath values can overwrite newer results. To
fix this, add a cleanup function that tracks whether the current effect is still
active, immediately call extensionHost.setKeystrokeEvents([]) at the start of
the effect, and modify the promise chain to only apply results if the effect has
not been cancelled (by checking a flag set in the cleanup function before
calling extensionHost.setKeystrokeEvents() in both the .then() and .catch()
handlers).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d35015b-d98e-4e0c-a091-0dfcf6688e5e
📒 Files selected for processing (14)
electron-builder.json5electron/electron-env.d.tselectron/ipc/constants.tselectron/ipc/cursor/interaction.tselectron/ipc/cursor/telemetry.tselectron/ipc/recording/mac.tselectron/ipc/register/recording.tselectron/ipc/state.tselectron/ipc/types.tselectron/ipc/utils.tselectron/preload.tspublic/builtin-extensions/keystrokes/index.jspublic/builtin-extensions/keystrokes/recordly-extension.jsonsrc/components/video-editor/VideoEditor.tsx
Comment on lines
+370
to
+377
| return parsed.events.filter( | ||
| (e: unknown): e is KeystrokeEvent => | ||
| e !== null && | ||
| typeof e === "object" && | ||
| typeof (e as KeystrokeEvent).timeMs === "number" && | ||
| typeof (e as KeystrokeEvent).key === "string" && | ||
| Array.isArray((e as KeystrokeEvent).modifiers), | ||
| ); |
Contributor
There was a problem hiding this comment.
Tighten modifier element validation in loader.
Line 376 only checks that modifiers is an array; non-string elements still pass and can propagate malformed telemetry across the IPC boundary.
Suggested fix
return parsed.events.filter(
(e: unknown): e is KeystrokeEvent =>
e !== null &&
typeof e === "object" &&
typeof (e as KeystrokeEvent).timeMs === "number" &&
typeof (e as KeystrokeEvent).key === "string" &&
- Array.isArray((e as KeystrokeEvent).modifiers),
+ Array.isArray((e as KeystrokeEvent).modifiers) &&
+ (e as KeystrokeEvent).modifiers.every((m: unknown) => typeof m === "string"),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return parsed.events.filter( | |
| (e: unknown): e is KeystrokeEvent => | |
| e !== null && | |
| typeof e === "object" && | |
| typeof (e as KeystrokeEvent).timeMs === "number" && | |
| typeof (e as KeystrokeEvent).key === "string" && | |
| Array.isArray((e as KeystrokeEvent).modifiers), | |
| ); | |
| return parsed.events.filter( | |
| (e: unknown): e is KeystrokeEvent => | |
| e !== null && | |
| typeof e === "object" && | |
| typeof (e as KeystrokeEvent).timeMs === "number" && | |
| typeof (e as KeystrokeEvent).key === "string" && | |
| Array.isArray((e as KeystrokeEvent).modifiers) && | |
| (e as KeystrokeEvent).modifiers.every((m: unknown) => typeof m === "string"), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/cursor/telemetry.ts` around lines 370 - 377, The filter function
that validates KeystrokeEvent objects is only checking that the modifiers
property is an array, but not validating that each element within the array is a
string. This allows malformed telemetry with non-string modifier values to pass
through the IPC boundary. Add an additional validation check to the filter
condition that ensures every element in the modifiers array is of type string,
in addition to the existing Array.isArray check on line 376.
Comment on lines
+1003
to
+1008
| try { | ||
| await saveKeystrokeTelemetry(finalVideoPath); | ||
| } catch (error) { | ||
| console.warn("Failed to persist keystroke telemetry during native stop:", error); | ||
| } | ||
| resetKeystrokeTelemetry(); |
Contributor
There was a problem hiding this comment.
Reset timing in Windows stop flow can delete the just-saved keystroke sidecar.
Line 1008 clears keystrokes before the later finalizeStoredVideo call in mux (Line 1473). That finalizer saves keystrokes again, and with an empty buffer it takes the delete-on-empty branch, removing ${videoPath}.keys.json. It also leaves terminal stop-failure paths without a guaranteed reset.
Suggested direction
try {
await saveKeystrokeTelemetry(finalVideoPath);
} catch (error) {
console.warn("Failed to persist keystroke telemetry during native stop:", error);
}
- resetKeystrokeTelemetry();
+ // Defer keystroke reset until finalizeStoredVideo() after mux/finalization.Also add a reset in terminal failure exits that do not reach finalizeStoredVideo() to prevent stale carryover.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/register/recording.ts` around lines 1003 - 1008, The
resetKeystrokeTelemetry() call at the native stop flow is executed before
finalizeStoredVideo() is invoked later in the mux flow (around line 1473), which
causes the keystroke buffer to be cleared prematurely. When
finalizeStoredVideo() subsequently attempts to save keystrokes and encounters an
empty buffer, it triggers the delete-on-empty branch and removes the keystroke
sidecar file that was just saved. Move the resetKeystrokeTelemetry() call to
execute after finalizeStoredVideo() completes, and also add
resetKeystrokeTelemetry() calls to all terminal failure exit paths that do not
reach finalizeStoredVideo() to prevent stale keystroke data from persisting
across multiple recordings.
Comment on lines
+3341
to
+3353
| useEffect(() => { | ||
| if (!videoSourcePath) { | ||
| extensionHost.setKeystrokeEvents([]); | ||
| return; | ||
| } | ||
| window.electronAPI | ||
| .getKeystrokeTelemetry(videoSourcePath) | ||
| .then((result: { success: boolean; events: Array<{ timeMs: number; key: string; modifiers: string[] }> }) => { | ||
| extensionHost.setKeystrokeEvents(result.success ? result.events : []); | ||
| }) | ||
| .catch(() => extensionHost.setKeystrokeEvents([])); | ||
| }, [videoSourcePath]); | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "VideoEditor.tsx" -type fRepository: webadderallorg/Recordly
Length of output: 113
🏁 Script executed:
wc -l src/components/video-editor/VideoEditor.tsxRepository: webadderallorg/Recordly
Length of output: 116
🏁 Script executed:
sed -n '3341,3353p' src/components/video-editor/VideoEditor.tsxRepository: webadderallorg/Recordly
Length of output: 507
🏁 Script executed:
# Check if extensionHost.setKeystrokeEvents is called elsewhere and in what context
rg "setKeystrokeEvents" src/components/video-editor/VideoEditor.tsx -B 2 -A 2Repository: webadderallorg/Recordly
Length of output: 489
🏁 Script executed:
# Search for extensionHost definition/initialization
rg "extensionHost" src/components/video-editor/VideoEditor.tsx | head -20Repository: webadderallorg/Recordly
Length of output: 689
🏁 Script executed:
# Look at the imports and top of the file to understand extensionHost
head -100 src/components/video-editor/VideoEditor.tsx | grep -E "extensionHost|import"Repository: webadderallorg/Recordly
Length of output: 1427
🏁 Script executed:
# More comprehensive search for extensionHost in VideoEditor
rg "extensionHost" src/components/video-editor/VideoEditor.tsx -B 0 -A 0 | wc -lRepository: webadderallorg/Recordly
Length of output: 70
🏁 Script executed:
# Search for extensionHost across the codebase
rg "extensionHost\s*=" src/ --type=tsx --type=ts -B 2 | head -50Repository: webadderallorg/Recordly
Length of output: 95
🏁 Script executed:
# Let's check if there are other useEffect hooks with similar async patterns to see if they use cancellation
rg "useEffect.*\(" src/components/video-editor/VideoEditor.tsx -A 10 | head -80Repository: webadderallorg/Recordly
Length of output: 2366
🏁 Script executed:
# Check for AbortController or cancellation patterns in the file
rg "AbortController|cancelled" src/components/video-editor/VideoEditor.tsxRepository: webadderallorg/Recordly
Length of output: 344
🏁 Script executed:
# Let's get context around the keystroke effect - 10 lines before and after
sed -n '3335,3360p' src/components/video-editor/VideoEditor.tsxRepository: webadderallorg/Recordly
Length of output: 900
🏁 Script executed:
# Search for how videoSourcePath is used and if it's truly dynamic
rg "videoSourcePath" src/components/video-editor/VideoEditor.tsx -B 1 -A 1 | head -40Repository: webadderallorg/Recordly
Length of output: 1310
🏁 Script executed:
# Check if window.electronAPI.getKeystrokeTelemetry is defined/used elsewhere
rg "getKeystrokeTelemetry" . --type=ts --type=tsx -B 2 -A 2Repository: webadderallorg/Recordly
Length of output: 95
🏁 Script executed:
# Let's look at the actual current code more carefully
sed -n '3340,3355p' src/components/video-editor/VideoEditor.tsx | cat -nRepository: webadderallorg/Recordly
Length of output: 709
Guard against stale keystroke telemetry responses when videoSourcePath changes.
This async effect can apply out-of-order results (old source overwriting new source), which can show/export wrong keystrokes. Clear events immediately and ignore stale completions via cleanup cancellation.
🔧 Suggested fix
useEffect(() => {
+ let cancelled = false;
+ extensionHost.setKeystrokeEvents([]);
+
if (!videoSourcePath) {
- extensionHost.setKeystrokeEvents([]);
- return;
+ return () => {
+ cancelled = true;
+ };
}
- window.electronAPI
+
+ void window.electronAPI
.getKeystrokeTelemetry(videoSourcePath)
- .then((result: { success: boolean; events: Array<{ timeMs: number; key: string; modifiers: string[] }> }) => {
- extensionHost.setKeystrokeEvents(result.success ? result.events : []);
+ .then((result) => {
+ if (cancelled) return;
+ extensionHost.setKeystrokeEvents(result.success ? result.events : []);
})
- .catch(() => extensionHost.setKeystrokeEvents([]));
+ .catch(() => {
+ if (!cancelled) extensionHost.setKeystrokeEvents([]);
+ });
+
+ return () => {
+ cancelled = true;
+ };
}, [videoSourcePath]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/VideoEditor.tsx` around lines 3341 - 3353, The
useEffect hook fetching keystroke telemetry via
window.electronAPI.getKeystrokeTelemetry() has a race condition where stale
responses from previous videoSourcePath values can overwrite newer results. To
fix this, add a cleanup function that tracks whether the current effect is still
active, immediately call extensionHost.setKeystrokeEvents([]) at the start of
the effect, and modify the promise chain to only apply results if the effect has
not been cancelled (by checking a flag set in the cleanup function before
calling extensionHost.setKeystrokeEvents() in both the .then() and .catch()
handlers).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Records keyboard input during capture and shows pressed keys as on-screen
keycaps during playback and export — like keyviz.
Ships as a built-in extension (
Keystrokes) that auto-activates; its settingslive under the cursor section.
Why
getKeystrokesInRange()already existed in the extension API surface, butnothing ever populated it —
setKeystrokeEvents()was never called, so italways returned
[]. This wires up the capture side and exposes the data.How
Capture (main process) — extends the existing
uiohook-napiinteractioncapture (already used for mouse) with
keydownevents:<video>.keys.jsonsidecar on stop, mirroring the.cursor.jsonpattern.shiftKey/ctrlKey/altKey/metaKeybooleans; bare modifier presses are not emitted on their own.
get-keystroke-telemetry+ preload binding; the editor loads thesidecar and feeds
extensionHost.setKeystrokeEvents().Overlay (built-in extension) —
public/builtin-extensions/keystrokes:key positions, so this remaps to the user's layout.
Notes / limitations
it is not a full per-layout character translation (e.g. AZERTY number-row
symbols). Knob is exposed so users can pick their layout.
extraResourcesentry.the diff (local build artifacts).
Related
foundation — the
.keys.jsonsidecar produced here could drive event-syncedtyping sounds.
Files
electron/ipc/cursor/interaction.tselectron/ipc/cursor/telemetry.tselectron/ipc/recording/mac.ts,register/recording.tselectron/ipc/{types,state,constants,utils}.tselectron/preload.ts,electron-env.d.tsgetKeystrokeTelemetrysrc/components/video-editor/VideoEditor.tsxpublic/builtin-extensions/keystrokes/*electron-builder.json5🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes