Skip to content

Bugfix: only round to the nearest cell for selection - #18486

Merged
Carlos Zamora (carlos-zamora) merged 2 commits into
mainfrom
dev/cazamor/bugfix-selection-rounding
Jan 31, 2025
Merged

Bugfix: only round to the nearest cell for selection#18486
Carlos Zamora (carlos-zamora) merged 2 commits into
mainfrom
dev/cazamor/bugfix-selection-rounding

Conversation

@carlos-zamora

Copy link
Copy Markdown
Member

Summary of the Pull Request

Fixes a bug where VT mouse mode would round to the nearest cell when clicking the mouse button.
The fix is to round to the nearest cell only when we're selecting text. The other scenarios affected are:

  • clicking on a hyperlink
  • vt mouse mode
  • where the context menu is anchored

Really the most notable ones were the first two. So now, we use the position of the cell we clicked on. We only round for selection.

References and Relevant Issues

Follow-up to #18106

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Opened Midnight Commander in Ubuntu and clicked between the two panes.

  • Before: threshold was too early to switch between panes
  • After: threshold is clearly separated between the outline of the two panes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do away with the GH# references - in a way, it's like saying "this is related to the selection changes". Well, every change here is related to selection ;)

Comment thread src/cascadia/TerminalControl/ControlInteractivity.cpp Outdated
Comment thread src/cascadia/TerminalControl/ControlInteractivity.cpp Outdated
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 31, 2025
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
@microsoft-github-policy-service microsoft-github-policy-service Bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 31, 2025
@carlos-zamora
Carlos Zamora (carlos-zamora) deleted the dev/cazamor/bugfix-selection-rounding branch January 31, 2025 22:30
Dustin L. Howett (DHowett) pushed a commit that referenced this pull request Feb 26, 2025
…mode (#18602)

Missed a few `_getTerminalPosition()` on the first run. Disabled
rounding for pointer movements and mouse wheel events (which are used
for hyperlink hover detection and vt mouse mode). The only time we round
now is...
- `SetEndSelectionPoint()` --> because we're updating a selection
- `ControlCore->LeftClickOnTerminal()` --> where all paths are used for
selection*

*the only path that doesn't is `RepositionCursorWithMouse` being
enabled, which also makes sense based on clicking around Notepad with a
large font size.

## References and Relevant Issues
Follow-up for #18486 
Closes #18595 

## Validation Steps Performed
In large font size, play around with midnight commander and hover over
hyperlink edges.
Dustin L. Howett (DHowett) pushed a commit that referenced this pull request Feb 26, 2025
…mode (#18602)

Missed a few `_getTerminalPosition()` on the first run. Disabled
rounding for pointer movements and mouse wheel events (which are used
for hyperlink hover detection and vt mouse mode). The only time we round
now is...
- `SetEndSelectionPoint()` --> because we're updating a selection
- `ControlCore->LeftClickOnTerminal()` --> where all paths are used for
selection*

*the only path that doesn't is `RepositionCursorWithMouse` being
enabled, which also makes sense based on clicking around Notepad with a
large font size.

## References and Relevant Issues
Follow-up for #18486
Closes #18595

## Validation Steps Performed
In large font size, play around with midnight commander and hover over
hyperlink edges.

(cherry picked from commit e5b972a)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgXgv1c
Service-Version: 1.23
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.

3 participants