Skip to content
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

Merged
merged 2 commits into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5143.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix location rendering in timeline if map cannot be loaded
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,17 @@

package im.vector.app.features.home.room.detail.timeline.item

import android.graphics.drawable.Drawable
import android.widget.ImageView
import android.widget.TextView
import androidx.core.view.isVisible
import com.airbnb.epoxy.EpoxyAttribute
import com.airbnb.epoxy.EpoxyModelClass
import com.bumptech.glide.load.DataSource
import com.bumptech.glide.load.engine.GlideException
import com.bumptech.glide.request.RequestListener
import com.bumptech.glide.request.RequestOptions
import com.bumptech.glide.request.target.Target
import im.vector.app.R
import im.vector.app.core.glide.GlideApp
import im.vector.app.features.home.room.detail.timeline.helper.LocationPinProvider
Expand All @@ -46,20 +53,32 @@ abstract class MessageLocationItem : AbsMessageItem<MessageLocationItem.Holder>(
GlideApp.with(holder.staticMapImageView)
.load(location)
.apply(RequestOptions.centerCropTransform())
.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)
Copy link
Member

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, thanks!

holder.staticMapErrorTextView.isVisible = true
return false
}

locationPinProvider?.create(locationOwnerId) { pinDrawable ->
GlideApp.with(holder.staticMapPinImageView)
.load(pinDrawable)
.into(holder.staticMapPinImageView)
}
override fun onResourceReady(resource: Drawable?, model: Any?, target: Target<Drawable>?, dataSource: DataSource?, isFirstResource: Boolean): Boolean {
locationPinProvider?.create(locationOwnerId) { pinDrawable ->
GlideApp.with(holder.staticMapPinImageView)
.load(pinDrawable)
.into(holder.staticMapPinImageView)
}
holder.staticMapErrorTextView.isVisible = false
return false
}
})
.into(holder.staticMapImageView)
}

override fun getViewType() = STUB_ID

class Holder : AbsMessageItem.Holder(STUB_ID) {
val staticMapImageView by bind<ImageView>(R.id.staticMapImageView)
val staticMapPinImageView by bind<ImageView>(R.id.staticMapPinImageView)
val staticMapErrorTextView by bind<TextView>(R.id.staticMapErrorTextView)
}

companion object {
Expand Down
9 changes: 9 additions & 0 deletions vector/src/main/res/drawable/ic_location_pin_failed.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="70dp"
android:height="70dp"
android:viewportWidth="70"
android:viewportHeight="70">
<path
android:pathData="M34.9997,5.8335C23.7122,5.8335 14.583,15.2112 14.583,26.8059C14.583,39.2995 27.4747,56.5269 32.783,63.0882C33.9497,64.5264 36.0788,64.5264 37.2455,63.0882C42.5247,56.5269 55.4163,39.2995 55.4163,26.8059C55.4163,15.2112 46.2872,5.8335 34.9997,5.8335ZM34.9997,34.2961C30.9747,34.2961 27.708,30.9405 27.708,26.8059C27.708,22.6714 30.9747,19.3158 34.9997,19.3158C39.0247,19.3158 42.2913,22.6714 42.2913,26.8059C42.2913,30.9405 39.0247,34.2961 34.9997,34.2961Z"
android:fillColor="#C1C6CD"/>
</vector>
13 changes: 13 additions & 0 deletions vector/src/main/res/layout/item_timeline_event_location_stub.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

<com.google.android.material.card.MaterialCardView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:cardCornerRadius="8dp">
Expand All @@ -21,4 +22,16 @@
android:importantForAccessibility="no"
android:src="@drawable/bg_map_user_pin" />

<TextView
android:id="@+id/staticMapErrorTextView"
style="@style/Widget.Vector.TextView.Subtitle"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal|bottom"
android:layout_marginBottom="54dp"
android:text="@string/location_timeline_failed_to_load_map"
android:textColor="?vctr_content_tertiary"
android:visibility="gone"
tools:visibility="visible" />

</com.google.android.material.card.MaterialCardView>
1 change: 1 addition & 0 deletions vector/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3756,6 +3756,7 @@
<string name="settings_enable_location_sharing">Enable location sharing</string>
<string name="settings_enable_location_sharing_summary">Once enabled you will be able to send your location to any room</string>
<string name="labs_render_locations_in_timeline">Render user locations in the timeline</string>
<string name="location_timeline_failed_to_load_map">Failed to load map</string>

<string name="tooltip_attachment_photo">Open camera</string>
<string name="tooltip_attachment_gallery">Send images and videos</string>
Expand Down