-
Notifications
You must be signed in to change notification settings - Fork 139
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
fix linkify #2040
fix linkify #2040
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
37ba174
to
d097495
Compare
TimelineItemEmoteContent( | ||
body = emoteBody, | ||
htmlDocument = messageType.formatted?.toHtmlDocument(prefix = "* $senderDisplayName"), | ||
formattedBody = parseHtml(messageType.formatted, prefix = "* $senderDisplayName") ?: emoteBody.withLinks(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, we always have a formattedBody
now with those fixes. Could we simplify a bit the TimelineItemTextBasedContent
interface then?
It's a bit hard to know what to use currently, and we probably don't need all the fields.
sealed interface TimelineItemTextBasedContent : TimelineItemEventContent {
val body: String
val htmlDocument: Document?
val formattedBody: CharSequence?
val plainText: String
val isEdited: Boolean
val htmlBody: String?
get() = htmlDocument?.body()?.html()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we always have a
formattedBody
No, withLinks()
returns null
if no links are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we still try to simplify the api?
If we could get a plainText/formattedText/body/htmlBody
and remove the htmlDocument
instance?
I'd say, The formattedText
shouldn't be nullable and should always be displayed in timeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I would prefer to do that in a separate rework PR. OK for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as u want
Would we ever consider also detecting and formatting raw user/room mentions in this way too, for clients that only send unformatted bodies? |
consumeItemsUntilPredicate { it.showSyncSpinner } | ||
roomListService.postState(RoomListService.State.Running) | ||
roomListService.postSyncIndicator(RoomListService.SyncIndicator.Hide) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consumeItemsUntilPredicate
now expects the predicate to be true at some point, so I had to fix this outdated test.
…f Complete occurs. Do not fail on error for `consumeItemsUntilTimeout`
4e1ba31
to
bd03831
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2040 +/- ##
===========================================
- Coverage 67.64% 67.61% -0.03%
===========================================
Files 1349 1349
Lines 33813 33819 +6
Branches 7263 7268 +5
===========================================
- Hits 22872 22867 -5
- Misses 7493 7503 +10
- Partials 3448 3449 +1 ☔ View full report in Codecov by Sentry. |
Type of change
Content
Ensure message with body and without formatted body got working links.
Also ensure email addresses are linkified.
Motivation and context
Closes #2038
Screenshots / GIFs
Tests
Tested devices
Checklist