fix(editor): dismiss git-blame tooltip on keydown (MACRO-2664) - #5200
fix(editor): dismiss git-blame tooltip on keydown (MACRO-2664)#5200jbecke wants to merge 2 commits into
Conversation
The per-line git-blame hover overlay only dismissed on pointerleave/pointerdown, so if it was showing from a prior hover it kept floating over the text while the user started typing in that spot. Add a KEY_DOWN_COMMAND handler that dismisses it the same way, reusing the existing dismiss() callback.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe blame tooltip plugin now dismisses on Lexical’s low-priority 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
`@apps/web/src/lib/core/component/LexicalMarkdown/plugins/blame-tooltip/blameTooltipPlugin.test.ts`:
- Around line 17-31: Update createTestEditor and the test setup to retain the
plugin teardown returned during registration, and add per-test cleanup that
unregisters the plugin, removes the editor root from document.body, and disposes
the Lexical editor. Ensure cleanup runs after each test so mounted DOM nodes and
listeners do not persist.
In
`@apps/web/src/lib/core/component/LexicalMarkdown/plugins/blame-tooltip/blameTooltipPlugin.ts`:
- Around line 104-114: Update the blame tooltip plugin’s dismissal handling
around the KEY_DOWN_COMMAND registration to also dismiss on composition start or
input events, covering IME paths where KEY_DOWN_COMMAND is skipped. Add a
regression test in blameTooltipPlugin.test.ts alongside the existing keyboard
dismissal tests that shows a tooltip is dismissed when composition begins.
- Around line 104-114: Update the tooltip plugin’s command registrations to also
handle COMPOSITION_START_COMMAND, calling dismiss() and returning false so
tooltips close when IME composition begins. Keep the existing KEY_DOWN_COMMAND
behavior unchanged, and add coverage for composition start in the blame tooltip
plugin test.
🪄 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: a049a2d4-fc43-40e3-b758-4eef68b5cfa4
📒 Files selected for processing (2)
apps/web/src/lib/core/component/LexicalMarkdown/plugins/blame-tooltip/blameTooltipPlugin.test.tsapps/web/src/lib/core/component/LexicalMarkdown/plugins/blame-tooltip/blameTooltipPlugin.ts
…p test Address CodeRabbit review on #5200: - KEY_DOWN_COMMAND is skipped while Lexical is composing (IME input), so the tooltip could keep showing through an entire composition. Add a COMPOSITION_START_COMMAND handler that dismisses it the same way. - Test now tears down the mounted editor/root and unregisters the plugin after each test, and covers the new composition-start path.
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
|
I think I already have a fix for this, but I'll take a look at this too |
|
Thanks for checking! I searched open PRs by you for anything blame-related and only found #4478 (sync-service refactor, unrelated). If you do have a fix for the dismiss-on-typing behavior specifically, happy to close this one in favor of yours — just point me at it. Otherwise this is a small, self-contained change (adds Generated by Claude Code |
4 participants
Summary
pointerleave/pointerdown.blameTooltipPlugin.tsonly wires uppointermove/pointerleave/pointerdownlisteners on the editor root; there's no keydown/input handling at all, so a tooltip left showing from a previous hover has nothing telling it to hide once the user starts editing that spot.KEY_DOWN_COMMANDhandler that calls the plugin's existingdismiss()callback, the same one already used forpointerleave/pointerdown.MACRO-2664
Why prioritized
Reported in bug-reports by Peter ("git blame should disappear when I start typing"), 👍'd by Wolf who owns the blame feature. Small, single-file, self-contained fix; no existing task/PR in flight.
Test plan
blameTooltipPlugin.test.ts: registers the plugin, dispatchesKEY_DOWN_COMMAND, and assertssetState({ hovering: false, nodeId: null })is called.bunx vitest run src/lib/core/component/LexicalMarkdown/plugins/blame-tooltip/blameTooltipPlugin.test.ts— 1 passed.bunx --bun biome checkon both changed files — clean.Note:
bun installfails in this sandbox on the privatetauri-plugins/pdf.jsgit deps, so I couldn't run the full dev server or project-wide type-check — I temporarily stubbed those two deps out locally to install everything else and run the real test suite, then reverted that before committing (nopackage.json/lockfile changes in this PR). Please give it a real hover-then-type test in dev before merging.Generated by Claude Code