Try avoiding trailing punctuation inside linkified URLs #4214
Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
| @Composable | ||
| internal fun ClickableLinkTextWithParenthesisPreview() = ElementThemedPreview { | ||
| ClickableLinkText( | ||
| annotatedString = AnnotatedString("Does this work (url: github.com/element-hq/element-x-android/README?)?!.", ParagraphStyle()), |
There was a problem hiding this comment.
Maybe add exemple where parenthesis is part of the url?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4214 +/- ##
===========================================
- Coverage 80.24% 80.23% -0.01%
===========================================
Files 2042 2043 +1
Lines 54079 54104 +25
Branches 6569 6580 +11
===========================================
+ Hits 43393 43412 +19
- Misses 8414 8415 +1
- Partials 2272 2277 +5 ☔ View full report in Codecov by Sentry. |
| end = adjustEndIndex(spannable, start, end) | ||
| }.onFailure { | ||
| Timber.e(it, "Failed to adjust end index for link span") | ||
| } |
There was a problem hiding this comment.
This is adjusting the end of the span, so this is fixing the rendering issue, but it does this affect the URL itself, i.e. the target of the link.
I think the line annotation = span.url, below needs to be updated.
There was a problem hiding this comment.
It's maybe because it did not work with the example I tried:
There was a problem hiding this comment.
Does the Event renderer use ClickableLinkText? It seems it's not the case 😅
There was a problem hiding this comment.
Ahhh damn, it used to use it, that's why I tried fixing it here. I'll move the code where it belongs 🥲 .
jmartinesp
force-pushed
the
fix/jme/avoid-trailing-punctuation-in-linkified-urls
branch
from
9ef2d3f to
323c811
Compare
|
|
This should be ready to review again: it's now affecting the intended texts (🤦 ) and has some extra unit tests for the new component. |
Content
When an URL is linkified automatically by
LinkifyCompat, try removing any trailing punctuation and other mismatched symbols from the link span.This solution is not great and might fail in some cases, but it should cover the most common ones, since it seems like a 'smart' linkification library is hard to find as a replacement.
Motivation and context
It turns out the
Linkifycomponent from Google isn't as smart as we thought: as soon as a/char is found inside the URL, it'll greedily fetch as much text as possible. This is probably because a different regex is used internally than for a simple domain.Should fix #4208.
Screenshots / GIFs
Tests
You can play with the
ClickableLinkTextWithParenthesisPreviewcomposable preview, or send messages with URLs and problematic formats to be linkified.Tested devices
Checklist