Skip to content

fix: keep composer mention highlights aligned while editing - #1211

Open
felipeggv wants to merge 2 commits into
RunMaestro:rcfrom
felipeggv:fix/prompt-editor-visual-alignment
Open

fix: keep composer mention highlights aligned while editing#1211
felipeggv wants to merge 2 commits into
RunMaestro:rcfrom
felipeggv:fix/prompt-editor-visual-alignment

Conversation

@felipeggv

@felipeggv felipeggv commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Keep native textarea glyphs and caret visible while the mention overlay paints decoration only.
  • Hide the decorative overlay during native text selection to prevent doubled selection rendering.
  • Synchronize the overlay scroll position after mount and text updates so mention highlights do not shift vertically.

Validation

  • Prettier check passed for both changed files.
  • TypeScript lint passed.
  • ESLint passed.
  • Focused InputArea suite: 129 passed.
  • Full Vitest suite: 33,907 passed, 108 skipped.
  • Renderer production build passed.
  • Manually verified cursor positioning, text selection, multiline scrolling, and agent mention alignment in Electron.

Summary by CodeRabbit

  • Bug Fixes
    • Improved mention highlighting in the input area.
    • Prevented overlay text from interfering with normal typing and text selection.
    • Temporarily hides mention decorations while selecting text and restores them afterward.
    • Improved overlay alignment and scrolling during input and initial display.
    • Ensured overlays appear only when recognized mentions are present.

@felipeggv

Copy link
Copy Markdown
Contributor Author

@jSydorowicz21 could you review this PR when available? It fixes the prompt editor cursor, selection, and mention-highlight alignment issues, includes regression coverage for the scrolled overlay case, and is rebased on the current rc. Local validation is green with 33,907 tests passed.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 74096a7c-c643-47c1-afba-9a383232d957

📥 Commits

Reviewing files that changed from the base of the PR and between 7deab10 and 9199dde.

📒 Files selected for processing (2)
  • src/__tests__/renderer/components/InputArea.test.tsx
  • src/renderer/components/InputArea/components/InputTextarea.tsx

📝 Walkthrough

Walkthrough

InputTextarea now conditionally renders transparent mention decorations, hides them during text selection, synchronizes overlay scrolling, and preserves native textarea text visibility. Tests cover unrecognized and recognized mentions, selection changes, and initial scroll alignment.

Changes

Mention overlay behavior

Layer / File(s) Summary
Conditional overlay rendering
src/renderer/components/InputArea/components/InputTextarea.tsx
Mention overlays render only for recognized mentions, remain transparent over native textarea text, and hide during active selection.
Selection and scroll synchronization
src/renderer/components/InputArea/components/InputTextarea.tsx
Textarea and document selection events update overlay visibility, while layout and scroll handlers synchronize overlay positioning.
Overlay behavior validation
src/__tests__/renderer/components/InputArea.test.tsx
Tests cover native styling, recognized-mention overlays, selection visibility, session-state reset, and initial scroll synchronization.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: keeping composer mention highlights aligned while editing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates the composer textarea mention overlay behavior. The main changes are:

  • Native textarea text and caret stay visible while the overlay paints decoration.
  • Mention overlay glyphs are made transparent to avoid doubled text.
  • The overlay is hidden during native text selection.
  • Newly mounted overlays sync their scroll position after render.
  • Focused tests cover mention rendering, selection behavior, and scroll alignment.

Confidence Score: 5/5

This looks safe to merge after a small overlay-state cleanup.

  • No blocking issues found in the changed code.
  • The only follow-up is a transient stale-selection state when the overlay unmounts and remounts around an old selection.

src/renderer/components/InputArea/components/InputTextarea.tsx

Important Files Changed

Filename Overview
src/renderer/components/InputArea/components/InputTextarea.tsx Updates the textarea and overlay layering, adds selection-state tracking, and syncs overlay scroll after mount.
src/tests/renderer/components/InputArea.test.tsx Adds focused coverage for native text visibility, recognized mention decoration, selection hiding, and scroll synchronization.

Reviews (1): Last reviewed commit: "fix: keep mention highlight aligned afte..." | Re-trigger Greptile

Comment on lines +104 to +105
const overlayRendered = overlayEnabled && segments.some((segment) => segment.kind !== 'text');
const overlayVisible = overlayRendered && !hasSelection;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale Selection Hides Overlay

When the user selects text, removes the only recognized mention so the overlay unmounts, then brings a mention back, hasSelection can still be true from the old selection. The remounted overlay starts with visibility: hidden until another selection or change event runs, so mention decoration can disappear even though there is no active selection.

@pedramamini

Copy link
Copy Markdown
Collaborator

@felipeggv Thanks for taking this on, and thanks for the unusually thorough validation write-up. Inverting the layering so the native textarea owns the glyphs, caret, and selection while the overlay paints decoration only is the right call. It kills a whole class of caret-drift bugs instead of papering over them, and the zero-inline-advance chip trick is preserved correctly. The four new focused tests are well targeted, too.

I found one issue I'd like fixed before we merge.

hasSelection is sticky across blur

hasSelection is only ever cleared by events on a focused textarea: onChange, onSelect, and the selectionchange listener, which early-returns via document.activeElement !== textarea. Nothing resets it on blur, and browsers preserve a textarea's selection range after it loses focus.

So this sequence leaves the decoration stuck off:

  1. Composer contains check @src/index.ts now, so the chip renders.
  2. Drag-select or double-click any word. hasSelection becomes true and the overlay goes visibility: hidden (intended).
  3. Click away, for example onto the Left Bar or any button. The textarea blurs but its selection range survives.
  4. hasSelection stays true, so the overlay stays hidden. The mention chip background and border are gone from an unfocused composer until the user clicks back in and collapses the caret.

I confirmed this against your branch with a throwaway test rather than by eyeballing it:

textarea.focus();
textarea.setSelectionRange(0, 5);
fireEvent(document, new Event('selectionchange'));
expect(overlay).toHaveStyle({ visibility: 'hidden' }); // passes

fireEvent.blur(textarea);
expect(overlay).toHaveStyle({ visibility: 'visible' }); // FAILS: still hidden

This is the same root cause as Greptile's P2 note about the stale flag surviving an overlay remount. Both come from hasSelection outliving the selection's relevance, so one fix covers both.

Suggested fix

Clear the flag on blur. Note onInputBlur is typed () => void, so it takes no argument:

onBlur={() => {
	setHasSelection(false);
	onInputBlur?.();
}}

I applied exactly this in a scratch worktree and ran the full InputArea.test.tsx suite: 130/130 pass, including all four of your new tests plus the repro above. A regression test for the blur path would be a welcome addition alongside it.

Non-blocking notes

  • CI: the test (windows-latest) leg is red, but it is not your fault. The only failure is AutoRunBlurSaveTiming.test.tsx > very long content can be saved timing out at 10s on the Windows runner, which this PR does not touch. We'll get it re-run. Both matrix legs do need to be green before this can land.
  • Chip text color: mentionChipStyle now hardcodes color: 'transparent', so the live composer chip always shows textMain glyphs from the textarea. No shipped theme sets mentionChipText today, so there is zero visible change, but a custom theme that sets it would see the composer chip and the sent-transcript pill diverge. Fine to leave as is; just flagging it.

No merge conflicts, and the branch is otherwise clean. Push the blur fix and I'll take another look. Nice work.

@pedramamini

Copy link
Copy Markdown
Collaborator

Thanks for this! Reviewed as part of a PR sweep. It currently conflicts with rc and the windows-latest test leg is red, so I could not merge it. Could you rebase on the latest rc and get both CI legs green? Happy to review and merge once it is clean.

@pedramamini

pedramamini commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@felipeggv Checking back in on this one. Thanks again for the work here, the layering inversion is still the approach we want and the new tests are good.

Nothing has landed on the branch since my last note on July 15, so the same three items are still open:

  1. Merge conflicts. The branch now conflicts with rc. Could you rebase onto the latest rc and resolve them so we get a clean merge?
  2. hasSelection sticky across blur. Still unaddressed on the current head: onBlur={onInputBlur} is passed through untouched, so nothing clears the flag when the textarea loses focus while a selection range survives. The mention chip decoration stays hidden on an unfocused composer. The one-line fix and repro are in my earlier comment. A regression test for the blur path alongside it would be great.
  3. CI. test (windows-latest) is red. The failure itself (AutoRunBlurSaveTiming.test.tsx) is not caused by this PR, but a rebase plus a re-run should clear it. Both matrix legs need to be green before this can land.

No rush, and no hard deadline on our side. Just let me know if you would rather hand it off and we can pick up the rebase and the blur fix ourselves. Happy to re-review as soon as it is pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants