-
Notifications
You must be signed in to change notification settings - Fork 155
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
RTL: ensure sender information are correctly rendered in the timeline #3681
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3681 +/- ##
===========================================
- Coverage 82.81% 82.81% -0.01%
===========================================
Files 1747 1747
Lines 41785 41789 +4
Branches 5109 5111 +2
===========================================
+ Hits 34604 34607 +3
Misses 5365 5365
- Partials 1816 1817 +1 ☔ View full report in Codecov by Sentry. |
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.
Check the remarks :)
) | ||
.clip(bubbleShape) | ||
.combinedClickable( | ||
onClick = onClick, | ||
onLongClick = onLongClick, | ||
indication = ripple(), | ||
interactionSource = interactionSource | ||
), | ||
) | ||
.onGloballyPositioned { coordinates -> |
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.
Using onGloballyPositioned is not really great for performance, also we already have the size in the drawWithContent
scope.
@@ -112,7 +120,7 @@ fun MessageEventBubble( | |||
drawCircle( | |||
color = Color.Black, | |||
center = Offset( | |||
x = 0f, | |||
x = if (isRtl) contentWidthPx else 0f, |
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.
Just x = if(isRtl) size.width else 0f
should be good!
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.
Of course, I do not know why I did not see that yesterday. Thanks!
Quality Gate passedIssues Measures |
Content
Sender information must be rendered at top start of the bubble.
In RTL, the cropping of the bubble was not correct (first commit) and the sender info was not aligned correctly (second commit)
Motivation and context
Second point of #2865
Screenshots / GIFs
From
TimelineItemEventRowTimestampPreview
:Tests
Tested devices
Checklist