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

Add word selection on double-tap #96

Merged

Conversation

mgazza
Copy link
Contributor

@mgazza mgazza commented Oct 23, 2024

Implement word selection functionality triggered by a double-tap. Words are identified as sequences of alphanumeric characters, while spaces and punctuation are ignored. Update the selection logic to highlight the word under the cursor and clear any existing selection. Adjust tests to validate word selection and ensure accurate behavior when tapping on non-alphanumeric characters.

@mgazza mgazza force-pushed the doubleclick-highlight-word branch 2 times, most recently from e27253f to 5bcf9ed Compare October 24, 2024 12:49
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Please avoid the self-evident comments. It's adding nearly 20% more lines to the source

It feels like some of this selection code could be factored out of the double tap handler method too, to keep methods shorter and more succinct - and less need for comments ;)

@mgazza
Copy link
Contributor Author

mgazza commented Oct 25, 2024

no problem i'll remove the comments :D

Implement word selection functionality triggered by a double-tap.
Words are identified as sequences of alphanumeric characters, while
spaces and punctuation are ignored. Update the selection logic to
highlight the word under the cursor and clear any existing selection.
Adjust tests to validate word selection and ensure accurate behavior
when tapping on non-alphanumeric characters.
@mgazza mgazza force-pushed the doubleclick-highlight-word branch from 5bcf9ed to 07c62e2 Compare October 25, 2024 06:37
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Looking good, sorry I missed a naming issue in last review

@mgazza mgazza requested a review from andydotxyz October 27, 2024 12:28
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks for this

@andydotxyz andydotxyz merged commit 02dbb6b into fyne-io:master Oct 28, 2024
5 checks passed
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.

2 participants