From 2ddb0782f3ad5bb35a2b56f1c4c9e8af88bd0e56 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 17 Jan 2023 13:44:24 +0100 Subject: [PATCH 1/6] fix to set placeholder images in chat Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/controllers/ChatController.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 98c3ef723..f44f3cf38 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -621,8 +621,8 @@ class ChatController(args: Bundle) : adapter = TalkMessagesListAdapter( senderId, messageHolders, - ImageLoader { imageView, url, _ -> - imageView.loadAvatarOrImagePreview(url!!, conversationUser, placeholder = payload as? Drawable) + ImageLoader { imageView, url, placeholder -> + imageView.loadAvatarOrImagePreview(url!!, conversationUser, placeholder as Drawable?) }, this ) From 5ba61482737548da03c93684f8b614e591c3eb23 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 17 Jan 2023 13:49:28 +0100 Subject: [PATCH 2/6] fix to immediately view placeholder + set min height without this fix, "getPayloadForImageLoader" was called before the placeholder was set (so it was null until getPayloadForImageLoader was called again after ~30seconds.). This is fixed by calling the super method at the end of "onBind". min height was set to avoid very tiny previews (like for the placeholders) Signed-off-by: Marcel Hibbe --- .../talk/adapters/messages/PreviewMessageViewHolder.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt index d2a41f1c1..e6d34a57b 100644 --- a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt +++ b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt @@ -105,7 +105,8 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : @SuppressLint("SetTextI18n") @Suppress("NestedBlockDepth", "ComplexMethod", "LongMethod") override fun onBind(message: ChatMessage) { - super.onBind(message) + image.minimumHeight = DisplayUtils.convertDpToPixel(MIN_IMAGE_HEIGHT, context).toInt() + time.text = dateUtils.getLocalTimeStringFromTimestamp(message.timestamp) if (userAvatar != null) { if (message.isGrouped || message.isOneToOneConversation) { @@ -228,6 +229,8 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : true, viewThemeUtils!! ) + + super.onBind(message) } private fun longClickOnReaction(chatMessage: ChatMessage) { @@ -341,5 +344,6 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : const val ACTOR_TYPE_BOTS = "bots" const val ACTOR_ID_CHANGELOG = "changelog" const val KEY_NAME = "name" + const val MIN_IMAGE_HEIGHT = 100F } } From 99c6d77b177c8f229965364f0a2744953d2bb358 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 17 Jan 2023 13:51:25 +0100 Subject: [PATCH 3/6] set fallback placeholder for loadImage set fallback placeholder if somehow null was passed as a placeholder Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/extensions/ImageViewExtensions.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/extensions/ImageViewExtensions.kt b/app/src/main/java/com/nextcloud/talk/extensions/ImageViewExtensions.kt index c8b391d22..0a487f405 100644 --- a/app/src/main/java/com/nextcloud/talk/extensions/ImageViewExtensions.kt +++ b/app/src/main/java/com/nextcloud/talk/extensions/ImageViewExtensions.kt @@ -149,13 +149,17 @@ fun ImageView.loadThumbnail(url: String, user: User): io.reactivex.disposables.D } fun ImageView.loadImage(url: String, user: User, placeholder: Drawable? = null): io.reactivex.disposables.Disposable { + var finalPlaceholder = placeholder + if (finalPlaceholder == null) { + finalPlaceholder = ContextCompat.getDrawable(context!!, R.drawable.ic_mimetype_file) + } val requestBuilder = ImageRequest.Builder(context) .data(url) .crossfade(true) .target(this) - .placeholder(placeholder) - .error(placeholder) + .placeholder(finalPlaceholder) + .error(finalPlaceholder) .transformations(RoundedCornersTransformation(ROUNDING_PIXEL, ROUNDING_PIXEL, ROUNDING_PIXEL, ROUNDING_PIXEL)) if (url.startsWith(user.baseUrl!!) && From 90d3d7d217c23d17c95525dec45e509f31f58bb9 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 17 Jan 2023 14:45:20 +0100 Subject: [PATCH 4/6] fix to show placeholder image if vcf contact has no photo Signed-off-by: Marcel Hibbe --- .../messages/PreviewMessageViewHolder.kt | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt index e6d34a57b..6329c7203 100644 --- a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt +++ b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt @@ -139,6 +139,7 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : messageText.text = fileName if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_NAME)) { previewContainer.visibility = View.GONE + previewContactContainer.visibility = View.VISIBLE previewContactName.text = message.selectedIndividualHashMap!![KEY_CONTACT_NAME] progressBar = previewContactProgressBar messageText.visibility = View.INVISIBLE @@ -146,17 +147,23 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : viewThemeUtils!!.talk.colorContactChatItemBackground(previewContactContainer) viewThemeUtils!!.talk.colorContactChatItemName(previewContactName) viewThemeUtils!!.platform.colorCircularProgressBarOnPrimaryContainer(previewContactProgressBar!!) + + if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_PHOTO)) { + image = previewContactPhoto + placeholder = getDrawableFromContactDetails( + context, + message.selectedIndividualHashMap!![KEY_CONTACT_PHOTO] + ) + } else { + image = previewContactPhoto + placeholder = ContextCompat.getDrawable(context!!, R.drawable.ic_mimetype_text_vcard) + } } else { previewContainer.visibility = View.VISIBLE previewContactContainer.visibility = View.GONE } - if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_PHOTO)) { - image = previewContactPhoto - placeholder = getDrawableFromContactDetails( - context, - message.selectedIndividualHashMap!![KEY_CONTACT_PHOTO] - ) - } else if (message.selectedIndividualHashMap!!.containsKey(KEY_MIMETYPE)) { + + if (message.selectedIndividualHashMap!!.containsKey(KEY_MIMETYPE)) { val mimetype = message.selectedIndividualHashMap!![KEY_MIMETYPE] val drawableResourceId = getDrawableResourceIdForMimeType(mimetype) val drawable = ContextCompat.getDrawable(context!!, drawableResourceId) @@ -258,10 +265,12 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : try { inputStream.close() } catch (e: IOException) { - val drawableResourceId = getDrawableResourceIdForMimeType("text/vcard") - drawable = ContextCompat.getDrawable(context, drawableResourceId) + Log.e(TAG, "failed to close stream in getDrawableFromContactDetails", e) } } + if (drawable == null) { + drawable = ContextCompat.getDrawable(context!!, R.drawable.ic_mimetype_text_vcard) + } return drawable } From ffcd56375e5ba6dee9b254d3c82129df8d7a6c24 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 17 Jan 2023 15:35:56 +0100 Subject: [PATCH 5/6] fix to hide avatars in one to one conversations. follow up to commit 7464e6994b673d2b575903a76752bf39bed9d622 the problem was that the "super" methods also contain logic to show/hide the avatar. So this result must be overwritten again after calling the super method. Signed-off-by: Marcel Hibbe --- .../messages/PreviewMessageViewHolder.kt | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt index 6329c7203..bb5f89221 100644 --- a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt +++ b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt @@ -108,28 +108,7 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : image.minimumHeight = DisplayUtils.convertDpToPixel(MIN_IMAGE_HEIGHT, context).toInt() time.text = dateUtils.getLocalTimeStringFromTimestamp(message.timestamp) - if (userAvatar != null) { - if (message.isGrouped || message.isOneToOneConversation) { - if (message.isOneToOneConversation) { - userAvatar.visibility = View.GONE - } else { - userAvatar.visibility = View.INVISIBLE - } - } else { - userAvatar.visibility = View.VISIBLE - userAvatar.setOnClickListener { v: View -> - if (payload is MessagePayload) { - (payload as MessagePayload).profileBottomSheet.showFor( - message.actorId!!, - v.context - ) - } - } - if (ACTOR_TYPE_BOTS == message.actorType && ACTOR_ID_CHANGELOG == message.actorId) { - userAvatar.loadChangelogBotAvatar() - } - } - } + viewThemeUtils!!.platform.colorCircularProgressBar(progressBar!!) clickView = image messageText.visibility = View.VISIBLE @@ -139,7 +118,6 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : messageText.text = fileName if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_NAME)) { previewContainer.visibility = View.GONE - previewContactContainer.visibility = View.VISIBLE previewContactName.text = message.selectedIndividualHashMap!![KEY_CONTACT_NAME] progressBar = previewContactProgressBar messageText.visibility = View.INVISIBLE @@ -237,7 +215,34 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : viewThemeUtils!! ) + // super.onBind is at this position, because: + // - it needs to be called after "placeholder" is set (otherwise placeholders are initially not loaded) + // - it needs to be before the show/hide logic is set (because super methods also have logic for this, which + // needs to be overwritten again) super.onBind(message) + + if (userAvatar != null) { + if (message.isGrouped || message.isOneToOneConversation) { + if (message.isOneToOneConversation) { + userAvatar.visibility = View.GONE + } else { + userAvatar.visibility = View.INVISIBLE + } + } else { + userAvatar.visibility = View.VISIBLE + userAvatar.setOnClickListener { v: View -> + if (payload is MessagePayload) { + (payload as MessagePayload).profileBottomSheet.showFor( + message.actorId!!, + v.context + ) + } + } + if (ACTOR_TYPE_BOTS == message.actorType && ACTOR_ID_CHANGELOG == message.actorId) { + userAvatar.loadChangelogBotAvatar() + } + } + } } private fun longClickOnReaction(chatMessage: ChatMessage) { From 4ccf8ac5a28e13a3a9f0095b7e0711251e876a4c Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 18 Jan 2023 10:17:53 +0100 Subject: [PATCH 6/6] move logic to getPayloadForImageLoader logic for setting the placeholder was moved to getPayloadForImageLoader. This is a better solution than in commit 9557bec9 where the onBind method had to be called in between other code. This is still not the best solution because getPayloadForImageLoader now contains more logic than it should (which is also not only responsible for the placeholder). Anyway as this is a hotfix it's the best solution for the moment. Signed-off-by: Marcel Hibbe --- .../messages/PreviewMessageViewHolder.kt | 102 +++++++++--------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt index bb5f89221..d6773ebf0 100644 --- a/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt +++ b/app/src/main/java/com/nextcloud/talk/adapters/messages/PreviewMessageViewHolder.kt @@ -105,6 +105,7 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : @SuppressLint("SetTextI18n") @Suppress("NestedBlockDepth", "ComplexMethod", "LongMethod") override fun onBind(message: ChatMessage) { + super.onBind(message) image.minimumHeight = DisplayUtils.convertDpToPixel(MIN_IMAGE_HEIGHT, context).toInt() time.text = dateUtils.getLocalTimeStringFromTimestamp(message.timestamp) @@ -116,53 +117,7 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : fileViewerUtils = FileViewerUtils(context!!, message.activeUser!!) val fileName = message.selectedIndividualHashMap!![KEY_NAME] messageText.text = fileName - if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_NAME)) { - previewContainer.visibility = View.GONE - previewContactName.text = message.selectedIndividualHashMap!![KEY_CONTACT_NAME] - progressBar = previewContactProgressBar - messageText.visibility = View.INVISIBLE - clickView = previewContactContainer - viewThemeUtils!!.talk.colorContactChatItemBackground(previewContactContainer) - viewThemeUtils!!.talk.colorContactChatItemName(previewContactName) - viewThemeUtils!!.platform.colorCircularProgressBarOnPrimaryContainer(previewContactProgressBar!!) - if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_PHOTO)) { - image = previewContactPhoto - placeholder = getDrawableFromContactDetails( - context, - message.selectedIndividualHashMap!![KEY_CONTACT_PHOTO] - ) - } else { - image = previewContactPhoto - placeholder = ContextCompat.getDrawable(context!!, R.drawable.ic_mimetype_text_vcard) - } - } else { - previewContainer.visibility = View.VISIBLE - previewContactContainer.visibility = View.GONE - } - - if (message.selectedIndividualHashMap!!.containsKey(KEY_MIMETYPE)) { - val mimetype = message.selectedIndividualHashMap!![KEY_MIMETYPE] - val drawableResourceId = getDrawableResourceIdForMimeType(mimetype) - val drawable = ContextCompat.getDrawable(context!!, drawableResourceId) - if (drawable != null && - ( - drawableResourceId == R.drawable.ic_mimetype_folder || - drawableResourceId == R.drawable.ic_mimetype_package_x_generic - ) - ) { - drawable.setColorFilter( - viewThemeUtils!!.getScheme(image.context).primary, - PorterDuff.Mode.SRC_ATOP - ) - } - placeholder = drawable - } else { - fetchFileInformation( - "/" + message.selectedIndividualHashMap!![KEY_PATH], - message.activeUser - ) - } if (message.activeUser != null && message.activeUser!!.username != null && message.activeUser!!.baseUrl != null @@ -215,12 +170,6 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : viewThemeUtils!! ) - // super.onBind is at this position, because: - // - it needs to be called after "placeholder" is set (otherwise placeholders are initially not loaded) - // - it needs to be before the show/hide logic is set (because super methods also have logic for this, which - // needs to be overwritten again) - super.onBind(message) - if (userAvatar != null) { if (message.isGrouped || message.isOneToOneConversation) { if (message.isOneToOneConversation) { @@ -254,6 +203,55 @@ abstract class PreviewMessageViewHolder(itemView: View?, payload: Any?) : } override fun getPayloadForImageLoader(message: ChatMessage?): Any? { + if (message!!.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_NAME)) { + previewContainer.visibility = View.GONE + previewContactContainer.visibility = View.VISIBLE + previewContactName.text = message.selectedIndividualHashMap!![KEY_CONTACT_NAME] + progressBar = previewContactProgressBar + messageText.visibility = View.INVISIBLE + clickView = previewContactContainer + viewThemeUtils!!.talk.colorContactChatItemBackground(previewContactContainer) + viewThemeUtils!!.talk.colorContactChatItemName(previewContactName) + viewThemeUtils!!.platform.colorCircularProgressBarOnPrimaryContainer(previewContactProgressBar!!) + + if (message.selectedIndividualHashMap!!.containsKey(KEY_CONTACT_PHOTO)) { + image = previewContactPhoto + placeholder = getDrawableFromContactDetails( + context, + message.selectedIndividualHashMap!![KEY_CONTACT_PHOTO] + ) + } else { + image = previewContactPhoto + image.setImageDrawable(ContextCompat.getDrawable(context!!, R.drawable.ic_mimetype_text_vcard)) + } + } else { + previewContainer.visibility = View.VISIBLE + previewContactContainer.visibility = View.GONE + } + + if (message.selectedIndividualHashMap!!.containsKey(KEY_MIMETYPE)) { + val mimetype = message.selectedIndividualHashMap!![KEY_MIMETYPE] + val drawableResourceId = getDrawableResourceIdForMimeType(mimetype) + val drawable = ContextCompat.getDrawable(context!!, drawableResourceId) + if (drawable != null && + ( + drawableResourceId == R.drawable.ic_mimetype_folder || + drawableResourceId == R.drawable.ic_mimetype_package_x_generic + ) + ) { + drawable.setColorFilter( + viewThemeUtils!!.getScheme(image.context).primary, + PorterDuff.Mode.SRC_ATOP + ) + } + placeholder = drawable + } else { + fetchFileInformation( + "/" + message.selectedIndividualHashMap!![KEY_PATH], + message.activeUser + ) + } + return placeholder }