-
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
Room list room summary cleanup #2273
Conversation
… to match RoomListRoomSummary constructor order.
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.
id has changed, and so the avatar color.
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.
no timestamp if no message.
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.
I'm fine with not displaying the timestamp if there's no last message, but DefaultLastMessageFormatter
has a CharSequence?
return type that could return null even if there is a last message formatted, so you'd see the room item at the top of the list when it's updated, but you wouldn't see a timestamp if I'm not mistaken, which would look weird.
Could we change this return type to CharSequence
and make sure we always return a String, even an empty one, if there is a last message for the room? So if there is a last event we always return something (maybe empty, but something) and if there really is nothing we return null
.
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.
I'm fine with not displaying the timestamp if there's no last message
I did not change any logic in the existing codebase, just wanted to have more coherent previews. But you're right, since DefaultLastMessageFormatter
can return null, we can have a room list item with no message and a timestamp. So the previous screenshot may reflect something that can happen.
if we want to change this, it should be done in a separate PR, as this one is only rework with no logical change.
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2273 +/- ##
===========================================
+ Coverage 70.04% 70.05% +0.01%
===========================================
Files 1335 1335
Lines 32674 32684 +10
Branches 6792 6793 +1
===========================================
+ Hits 22885 22896 +11
Misses 6512 6512
+ Partials 3277 3276 -1 ☔ View full report in Codecov by Sentry. |
Use `?.let {}` instead of if/else with a null block when checking for nullable values [UseLet]
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
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.
LGTM if the CI passes.
Type of change
Content
Following #2130, there were more cleanup to do.
Motivation and context
Be ready for #2265 to (be rebased and) land properly.
Screenshots / GIFs
Tests
Tested devices
Checklist