Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Triple click in text editor now uses last mouse position for validity #51498

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

codecat
Copy link
Contributor

@codecat codecat commented Aug 11, 2021

Previously, you would be able to double click a word, followed by single-clicking another word on the same line, which would select the entire line. Now, it will only select the whole line if the mouse position has remained the same after the double click. This mimics the behavior in most third party text editors.

This fixes #51312.

@codecat codecat requested a review from a team as a code owner August 11, 2021 08:00
@Chaosus Chaosus added this to the 4.0 milestone Aug 11, 2021
@Chaosus Chaosus added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 11, 2021
@akien-mga
Copy link
Member

akien-mga commented Aug 11, 2021

Shouldn't there be a tolerance of a few pixels for unintentional mouse movement?

I checked on Firefox/GTK3 on Linux and triple click select seems to work if I'm moving very slightly while still triple clicking fast. Not sure what the tolerance used is but it might 5 px or something.

@codecat
Copy link
Contributor Author

codecat commented Aug 11, 2021

You're correct, I've never noticed this! Although it doesn't seem to be the case on VSCode, it seems like everywhere else there is indeed a tolerance.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good! Could you squash the commits into one? See PR workflow for instructions.

scene/gui/text_edit.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@Ansraer Ansraer left a comment

Choose a reason for hiding this comment

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

Damn it, you beat me to it by a day...
I am just gonna blame my slow internet and the good weather.

@codecat
Copy link
Contributor Author

codecat commented Aug 11, 2021

Is the Linux CI failure something I should concern myself with?

@Calinou
Copy link
Member

Calinou commented Aug 11, 2021

Is the Linux CI failure something I should concern myself with?

The CI log says:

Run echo "Running --doctool to see if this changes the public API without updating the documentation."
Running --doctool to see if this changes the public API without updating the documentation.
If a diff is shown, it means that your code/doc changes are incomplete and you should update the class reference with --doctool.


diff --git a/doc/classes/@GlobalScope.xml b/doc/classes/@GlobalScope.xml
index 3dfb0f4..7c954df 100644
--- a/doc/classes/@GlobalScope.xml
+++ b/doc/classes/@GlobalScope.xml
@@ -213,10 +213,8 @@
 			</description>
 		</method>
 		<method name="error_string">
-			<return type="String">
-			</return>
-			<argument index="0" name="error" type="int">
-			</argument>
+			<return type="String" />
+			<argument index="0" name="error" type="int" />
 			<description>
 				Returns a human-readable name for the given error code.
 			</description>
Error: Process completed with exit code 1.

You need to update the class reference to resolve this. To do so, compile an editor binary then run it with --doctool from the root Godot folder.

Edit: This is an issue with a PR that was merged today, and was fixed by #51523 since. I recommend rebasing on the latest master branch to resolve it.

Previously, you would be able to double click a word, followed by
single-clicking another word on the same line, which would select the
entire line. Now, it will only select the whole line if the mouse
position has remained the same after the double click. This mimicks the
behavior in most third party text editors.

Fixes godotengine#51312.
@akien-mga akien-mga merged commit 7188cb6 into godotengine:master Aug 11, 2021
@akien-mga
Copy link
Member

akien-mga commented Aug 11, 2021

Thanks! And congrats for your first merged Godot contribution 🎉

@codecat codecat deleted the fix-triple-click branch August 11, 2021 22:17
@akien-mga
Copy link
Member

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 12, 2021
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.

Triple click doesn't take mouse position into account
6 participants