Skip to content

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

Merged
carlos-zamora merged 2 commits intomainfrom
dev/cazamor/bugfix-selection-rounding
Jan 31, 2025
Merged

Bugfix: only round to the nearest cell for selection#18486
carlos-zamora merged 2 commits intomainfrom
dev/cazamor/bugfix-selection-rounding

Conversation

@carlos-zamora
Copy link
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
Member

@DHowett DHowett left a comment

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 ;)

@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 <[email protected]>
@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 enabled auto-merge (squash) January 31, 2025 22:08
@carlos-zamora carlos-zamora merged commit 425d6b0 into main Jan 31, 2025
17 of 19 checks passed
@carlos-zamora carlos-zamora deleted the dev/cazamor/bugfix-selection-rounding branch January 31, 2025 22:30
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.
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