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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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.
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

position.go Outdated Show resolved Hide resolved
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