fix(desktop): open DiffPane comment composer on pointer-up

[Kitenite]

Summary

• The inline agent-comment composer on the v2 DiffPane was popping open the moment the user moused down on a line and following the drag. Pierre's top-level onSelectedLinesChange fires for every selection notify (start, drag, end), which is why.
• Switch to the per-item onLineSelectionEnd callback, which only fires from pierre's handleDocumentPointerUp. The composer now appears once on mouse-up with the committed range.
• onGutterUtilityClick is reduced to a required non-null stub — pierre gates the "+" button's pointer flow on a non-null handler, but the gutter pointer-up path already fires notifySelectionEnd → onLineSelectionEnd, so the open is handled there.

onLineSelectionEnd chosen over onLineSelected because the latter also fires on imperative selection writes (clearSelectedLines, future jump-to-thread, etc.), which would re-open the composer spuriously.

Test plan

• Drag-select a range in a diff file → composer appears only on mouse-up at the committed end line.
• Click a single line → composer appears on click release (single Start+End pointer cycle).
• Click the gutter "+" button → composer opens on release.
• Re-click the same selected line → selection clears, composer closes.
• Submit a comment → clearSelectedLines runs; composer does not re-open from the imperative clear.

---

---

Summary by cubic

Fixes the DiffPane inline comment composer so it opens on mouse-up, not mouse-down, preventing it from popping during drag. Uses the per-item selection end signal to open once with the committed range.

• Bug Fixes
  • Switched to onLineSelectionEnd from @pierre/diffs to open the composer on pointer-up.
  • Removed reliance on the top-level selection change event to avoid start/drag and imperative reopen triggers.
  • Kept onGutterUtilityClick as a no-op stub; the gutter path already triggers selection end on release.

^{Written for commit 1349647. Summary will update on new commits.}

Review in cubic

Summary by CodeRabbit

• Refactor
  • Improved internal event handling for line selection in diff views.

[capy-ai]

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR migrates diff pane line selection handling from onSelectedLinesChange (drag events) to onLineSelectionEnd (pointer-up). The useDiffCommentComposer hook's return type, internal selection logic, and DiffPane consumer wiring are all updated to use the new pointer-up event for opening the composer.

Changes

Line Selection Signal Migration

Layer / File(s)	Summary
Hook callback contract and result type useDiffCommentComposer.ts	Hook imports CodeViewOptions instead of CodeViewLineSelection, and UseDiffCommentComposerResult exports onLineSelectionEnd (typed from CodeViewOptions["onLineSelectionEnd"]) instead of the previous selection-change callback.
Selection event handler and return value useDiffCommentComposer.ts	onLineSelectionEnd handler is added to set/clear composer state on pointer-up (closes when selection is null, opens when range exists and context is diff). The gutter click handler becomes a no-op stub. Hook return object exposes onLineSelectionEnd instead of onSelectedLinesChange.
DiffPane consumer wiring DiffPane.tsx	DiffPane destructures onLineSelectionEnd from the hook, passes it to codeViewOptions, updates memo dependencies, and removes the obsolete onSelectedLinesChange prop from the CodeView component.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• superset-sh/superset#4966: Refactored the v2 DiffPane inline agent-comment composer with the same useDiffCommentComposer hook updates for selection callback migration.

Poem

🐰 A pointer-up, selection's done!
No more drag events in the sun.
onLineSelectionEnd takes the throne,
The composer now has a cleaner home. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description lacks the required 'Description', 'Related Issues', 'Type of Change', 'Testing', and 'Additional Notes' sections from the template; it instead provides a custom summary followed by cubic and stage-review sections.	Restructure the description to follow the repository template with sections for Description, Related Issues, Type of Change, Testing, and Additional Notes.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(desktop): open DiffPane comment composer on pointer-up' directly describes the main change: switching to use onLineSelectionEnd to open the composer on mouse-up instead of on mouse-down.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch comment-menu-mouse-up

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[stage-review]

Ready to review this PR? Stage has broken it down into 3 individual chapters for you:

	Title
1	Update types for line selection end callback
2	Implement pointer-up logic in comment composer hook
3	Wire pointer-up selection to DiffPane component

_{Chapters generated by Stage for commit 23d097a on May 28, 2026 5:44pm UTC.}

[cubic-dev-ai]

No issues found across 2 files

Architecture diagram

sequenceDiagram
    participant User as User (Mouse)
    participant DiffPane as DiffPane Component
    participant Hook as useDiffCommentComposer Hook
    participant Pierre as @pierre/diffs Library
    participant State as DiffCommentComposer State

    Note over User,State: DiffPane Comment Composer Flow

    User->>DiffPane: Click/Pointer-down on diff line
    DiffPane->>Pierre: onLineSelectionStart (pass-through)
    Pierre->>Pierre: Track pointer session
    
    User->>DiffPane: Drag pointer to new line
    DiffPane->>Pierre: Notify selection change
    Pierre->>Pierre: Update internal selection
    
    Note over DiffPane,Pierre: Top-level onSelectedLinesChange (removed)<br/>NO LONGER fires composer open

    User->>DiffPane: Pointer-up / mouse-up
    Pierre->>Pierre: handleDocumentPointerUp
    Pierre->>Hook: onLineSelectionEnd(range, context)
    
    alt Valid range and context.type === "diff"
        Hook->>State: setComposer({ itemId, range })
        State-->>DiffPane: Render composer annotation
    else Null range (deselect)
        Hook->>State: setComposer(null)
        State-->>DiffPane: Close composer
    end

    Note over User,Pierre: Gutter "+" button path

    User->>DiffPane: Click gutter "+" button
    DiffPane->>Pierre: onGutterUtilityClick (no-op stub)
    Pierre->>Pierre: Start gutter pointer session (gated on non-null handler)
    Pierre->>Pierre: notifySelectionEnd (from pointer-up)
    Pierre->>Hook: onLineSelectionEnd(range, context)
    
    alt Range present
        Hook->>State: setComposer({ itemId, range })
        State-->>DiffPane: Open composer
    end

Loading

_{Re-trigger cubic}

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ⚠️ Neon database branch

Thank you for your contribution! 🎉

[greptile-apps]

Greptile encountered an error while reviewing this PR. Please reach out to support@greptile.com for assistance.