Repository navigation
Conversation
…corder
The workspace keydown handler ran
document.querySelector('[data-shortcut-recorder-active="true"]') before
anything else, on every key including IME composition keys. An attribute
selector has no fast path, so each keystroke walked the whole DOM, which
grows with the open transcripts.
The recorder input begins recording on focus and stops on blur, so the
recorder is active exactly when it is the focused element: check
document.activeElement instead.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description check The description includes all required sections, explains the change and reason, states that there is no UI change, and confirms the checklist items. It also documents validation results and the absenc… Title check The title clearly and concisely describes the main change: preventing each keystroke from querying the full page for the shortcut recorder. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes all required sections, explains the change and reason, states that there is no UI change, and confirms the checklist items. It also documents validation results and the absence of new tests.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
1 participant
What changed
The workspace keydown handler checks whether the focused element is the active shortcut recorder (
document.activeElement?.getAttribute("data-shortcut-recorder-active") === "true") instead of runningdocument.querySelector('[data-shortcut-recorder-active="true"]')on every key.Why
That query runs before anything else in the handler, on every keystroke, including IME composition keys. An attribute selector has no id/class fast path, so WebKit walks the whole DOM each time, and the DOM grows with every open transcript. In a profile of a real window while typing (macOS
sample/Time Profiler of the WebContent process, long sessions open), the query accounted for about 145 ms of a 10 s typing sample.The check is equivalent: the recorder is the
<input data-shortcut-recorder-active>in the keybinding settings row, which starts recording inonFocusand stops inonBlur, so it is active exactly when it is the focused element.UI
No visible change.
Validation
npm run check:web: vitest (5363 passed, 13 skipped) andtsc --noEmit. No Rust changes.Workspace's inline keydown effect, and the behaviour (the recorder still swallows chords while recording) is unchanged by construction. Happy to add one if you'd like it covered.Checklist
npm run check(check:web; no Rust files changed)Summary by CodeRabbit