Skip to content

Fix triple select edge case#3669

Merged
Tyriar merged 2 commits intoxtermjs:masterfrom
silamon:tripleselect
Mar 31, 2022
Merged

Fix triple select edge case#3669
Tyriar merged 2 commits intoxtermjs:masterfrom
silamon:tripleselect

Conversation

@silamon
Copy link
Copy Markdown
Contributor

@silamon silamon commented Mar 2, 2022

Keep the length of the (wrapped) line in selectionStartLength instead of selectionEnd. Upon dragging, the selection end is modified, but the wrapped line remains selected thanks to the selectionStartLength. ( Same behavior as selecting a word. )
Tests have been adjusted to include this.

Note: Dragging still select lines, instead of wrapped lines. EDIT: Fixed as well.

Fixes #3670
Related microsoft/vscode/#135567

See also #1456 (comment)

Copy link
Copy Markdown
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Looked into this and it still doesn't look quite right:
Recording 2022-03-02 at 06 11 13

Notice:

  • The flash when you drag up
  • The wrapped selection is only retained when you drag up
  • Dragging down doesn't keep the line selected

Ideally once triple click happens that initially selected wrapped line will always be selected. We're basically after exactly what monaco does:

Recording 2022-03-02 at 06 18 46

@silamon
Copy link
Copy Markdown
Contributor Author

silamon commented Mar 2, 2022

Looked into this and it still doesn't look quite right: Recording 2022-03-02 at 06 11 13

Notice:

  • The flash when you drag up
  • The wrapped selection is only retained when you drag up
  • Dragging down doesn't keep the line selected

Ideally once triple click happens that initially selected wrapped line will always be selected. We're basically after exactly what monaco does:

Recording 2022-03-02 at 06 18 46

That's exactly the note which was mentioned in the PR description. It's now also fixed on this branch.

@silamon silamon requested a review from Tyriar March 18, 2022 05:42
Copy link
Copy Markdown
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Works great 👍

@Tyriar Tyriar merged commit 5030dc3 into xtermjs:master Mar 31, 2022
@Tyriar Tyriar added this to the 4.19.0 milestone Mar 31, 2022
@silamon silamon deleted the tripleselect branch October 16, 2022 09:51
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.

Triple click and drag up/down should keep the initially clicked wrapped line selected

2 participants