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

enhancement: link clickable span detection #129

Closed

Conversation

diegoberaldin
Copy link

@diegoberaldin diegoberaldin commented Apr 25, 2024

Hi! I am using your library in my Lemmy client and I really like it (lightweight, easy to use, extensible, multiplatform to the core, perfect MD integration, etc.). Kudos!

My users reported the following bug: if there is a link inside a text node, the whole line is clickable and opens it even if the tapped position exceeds the anchor text length, i. e. provided that the tap event occurs in the same line after the link, the link is opened anyways.

Steps to reproduce: in the example application change the last two lines of the sample text like this

  [https://github.com/mikepenz](https://github.com/mikepenz), but this part of the line is not clickable
  [Mike Penz's Blog](https://blog.mikepenz.dev/) neither this part of the line should be clickable

The solution was a little more difficult than expected, because the LINK_DEFINITION node handler is not called for autolinks or links inside a text node, but just for internal Markdown references, so I had to modify even the text handlers. Another "weird" behaviour which I found is that annotations are added multiple times for the same reference, which was somehow "hidden" by the reversed().firstOrNull() access to the list, but the test on the hit rect against the annotation position had to be improved.

@mikepenz
Copy link
Owner

Thank you very much for the PR.
Will try to get it reviewed as soon as possible. had a quick peek, and yeah as you wrote it's quite a tricky change :D

@mikepenz mikepenz self-assigned this Apr 25, 2024
@diegoberaldin
Copy link
Author

Hi Mike, don't worry take your time to fully review it and making sure it does not break more things than it appears to fix. I hope I managed to explain what the issue was (a tap happening on the same line where a hyperlink is, but outside the text bounds on the "right" side).

PS. I hope I'll be able to open more PRs in the future, "my" users found some other minor issues and some of them can be addressed. Others are unmanageable in my opinion (because of Lemmy's Markdown dialect, e.g. spoilers) so I can work around them client-side in some way.

@mikepenz
Copy link
Owner

Thank you again for the PR. I identified a simpler solution for the problem, however I am not sure it solves all problems you noted. I will open a PR and get it merged. Would you be so kind to check it out with your usecases please?

mikepenz added a commit that referenced this pull request Apr 25, 2024
@mikepenz
Copy link
Owner

Please see here: #135

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