-
Notifications
You must be signed in to change notification settings - Fork 749
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 location rendering in timeline if map cannot be loaded #5145
Conversation
Matrix SDKIntegration Tests Results:
|
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, just one question (not blocking).
.into(holder.staticMapImageView) | ||
.listener(object : RequestListener<Drawable> { | ||
override fun onLoadFailed(e: GlideException?, model: Any?, target: Target<Drawable>?, isFirstResource: Boolean): Boolean { | ||
holder.staticMapPinImageView.setImageResource(R.drawable.ic_location_pin_failed) |
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.
This is maybe a design decision, but when the map fail to load, we could still draw the user avatar if available, no?
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.
CC @gaelledel
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 it is a design decision and it seems like a bug/ugly to use user avatar.
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 agree, thanks!
* develop: (54 commits) Bubbles: add CHANGELOG file Bubble: get LayoutDirection (RTL) from current Locale Version++ fastlane towncrier Version 1.3.18 changelog Improve missing state event detection to missing state events only one joined rooms (ignore LEFT room) Should reduce the number of initial sync Co-authors: ganfra and billcarsonfr Changelog added. taking the use case screen into account when accessing the sign up flows in the sanity tests updating copy split to match designs applying design feedback promoting use case strings for translation enabling the use case feature by default Code review fixes. ktlint Bubbles: clean up after review Sync: avoid deleting root event of CurrentState on gappy sync Code review fixes. Support generic location pin. ... # Conflicts: # vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/MessageLocationItem.kt # vector/src/main/res/layout/item_timeline_event_location_stub.xml
Fixes #5143
Figma