Skip to content

fix: use mouse modifiers for editor drag and drop - #335428

Open
Tiancheng Xu (Tiancheng-Xu) wants to merge 2 commits into
microsoft:mainfrom
Tiancheng-Xu:fix/58515-mouse-modifiers
Open

Tiancheng Xu (Tiancheng-Xu) wants to merge 2 commits into
microsoft:mainfrom
Tiancheng-Xu:fix/58515-mouse-modifiers

Conversation

@Tiancheng-Xu

Copy link
Copy Markdown

Fixes #58515

Summary

  • Use the modifier flags already carried by editor mouse drag/drop events.
  • Remove the keyboard listeners and cached mouse/modifier state from the drag-and-drop controller.
  • Preserve mouse-style reset and drop-cancel/blur cleanup behavior.

Validation

  • git diff --check
  • Full VS Code test suite not run locally because dependencies are not installed in this checkout; GitHub CI will provide the repository toolchain validation.

AI assistance was used to inspect the issue and prepare this focused change; the final diff and validation statement were manually checked.

Copilot AI balanced review requested due to automatic review settings September 10, 2026 09:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It alters user-facing editor drag-and-drop interaction semantics (dropping immediate copy-cursor feedback on modifier keypress without mouse movement), a UX trade-off that warrants human sign-off.

Pull request overview

This PR simplifies the editor's drag-and-drop controller (DragAndDropController) to read the trigger modifier (Alt on macOS, Ctrl elsewhere) directly from the modifier flags carried by editor mouse events, instead of separately tracking keyboard keydown/keyup events and a cached mouse-down/modifier state. This implements the request in issue #58515 and removes now-redundant bookkeeping.

Changes:

  • Narrows hasTriggerModifier to accept only IMouseEvent and removes the keyboard onKeyDown/onKeyUp listeners plus the _mouseDown, _modifierPressed, and TRIGGER_KEY_VALUE state.
  • Simplifies the drop condition and DragAndDropCommand copy flag to depend only on hasTriggerModifier(mouseEvent.event).
  • Drops the now-unused IKeyboardEvent, KeyCode, and EditorOption imports and the associated cleanup lines in blur/drop/dispose paths.
File summaries
File Description
src/vs/editor/contrib/dnd/browser/dnd.ts Uses mouse-event modifier flags for DnD copy behavior and removes keyboard listeners and cached mouse/modifier state.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Tiancheng-Xu

Copy link
Copy Markdown
Author

@Tiancheng-Xu

Copy link
Copy Markdown
Author

@Tiancheng-Xu

Copy link
Copy Markdown
Author

@Tiancheng-Xu

Copy link
Copy Markdown
Author

Thanks for the review. This PR intentionally removes the keyboard-state bookkeeping requested by #58515 and reads the trigger modifier from the editor mouse events. The copy semantics at mouse drag/drop remain driven by the event modifier; the only intentional UX change is that the cursor no longer reacts immediately to a modifier keypress without mouse movement. I'll leave this open for maintainer UX sign-off.

@Tiancheng-Xu

Copy link
Copy Markdown
Author

Following up on the UX trade-off I noted above. I see #324285 is still open and preserves the keyboard handlers for immediate cursor feedback, while this PR removes them. Would you prefer that I revise this PR to retain that feedback, or is the current behavior acceptable? I’ll leave the implementation unchanged until I have your guidance. I also see the required workflow checks are awaiting maintainer approval.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use the modifier properties on mouse event instead of tracking keydown/keyup

4 participants