From 7b32009cf548db8b1c45646fda1f7720b1b0bc75 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Mar 2024 16:26:22 +0200 Subject: [PATCH 1/8] Adds pop animation glide loading --- .../util/image/GlidePopTransitionOptions.kt | 41 +++++++++++++++++++ .../android/util/image/ImageManager.kt | 25 +++++++++++ 2 files changed, 66 insertions(+) create mode 100644 WordPress/src/main/java/org/wordpress/android/util/image/GlidePopTransitionOptions.kt diff --git a/WordPress/src/main/java/org/wordpress/android/util/image/GlidePopTransitionOptions.kt b/WordPress/src/main/java/org/wordpress/android/util/image/GlidePopTransitionOptions.kt new file mode 100644 index 000000000000..b24f546a0359 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/util/image/GlidePopTransitionOptions.kt @@ -0,0 +1,41 @@ +package org.wordpress.android.util.image + +import android.graphics.drawable.Drawable +import android.view.animation.AnimationUtils +import com.bumptech.glide.TransitionOptions +import com.bumptech.glide.load.DataSource +import com.bumptech.glide.request.transition.NoTransition +import com.bumptech.glide.request.transition.Transition +import com.bumptech.glide.request.transition.TransitionFactory +import org.wordpress.android.R +import org.wordpress.android.util.AppLog +import java.lang.RuntimeException + +object GlidePopTransitionOptions : TransitionOptions() { + fun pop(): GlidePopTransitionOptions { + return transition(GlidePopTransitionFactory()) + } +} + +class GlidePopTransition : Transition { + @Suppress("TooGenericExceptionCaught") + override fun transition(current: Drawable?, adapter: Transition.ViewAdapter?): Boolean { + adapter?.view?.context?.let { + adapter.setDrawable(current) + try { + val pop = AnimationUtils.loadAnimation(it, R.anim.pop) + adapter.view.startAnimation(pop) + } catch (e: RuntimeException) { + AppLog.e(AppLog.T.UTILS, "Error animating drawable: $e") + } + } + return true + } +} + +class GlidePopTransitionFactory : TransitionFactory { + private val transition: GlidePopTransition by lazy { GlidePopTransition() } + override fun build(dataSource: DataSource?, isFirstResource: Boolean): Transition { + return if (dataSource == DataSource.MEMORY_CACHE) NoTransition.get() else transition + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt b/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt index 4a2ba404c138..591bb748cf83 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt @@ -25,6 +25,7 @@ import androidx.core.content.ContextCompat import androidx.fragment.app.FragmentActivity import com.bumptech.glide.Glide import com.bumptech.glide.RequestBuilder +import com.bumptech.glide.TransitionOptions import com.bumptech.glide.load.DataSource import com.bumptech.glide.load.engine.GlideException import com.bumptech.glide.load.resource.bitmap.CenterCrop @@ -301,6 +302,30 @@ class ImageManager @Inject constructor( .clearOnDetach() } + /** + * Loads an image from the "imgUrl" into the ImageView animating it with the provided Glide animation. + * Adds a placeholder and an error placeholder depending on the ImageType and attaches a ResultListener. + */ + fun animateWithResultListener( + imageView: ImageView, + imageType: ImageType, + imgUrl: String, + transitionOptions: TransitionOptions<*, in Drawable>, + requestListener: RequestListener + ) { + val context = imageView.context + if (!context.isAvailable()) return + Glide.with(context) + .load(Uri.parse(imgUrl)) + .addFallback(imageType) + .addPlaceholder(imageType) + .applyScaleType(CENTER) + .attachRequestListener(requestListener) + .transition(transitionOptions) + .into(imageView) + .clearOnDetach() + } + /** * Preloads an [MShot]. * From 83b145502c6ed980a7f5f05d2e2a3fb920ee734c Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Mar 2024 16:27:06 +0200 Subject: [PATCH 2/8] Replaces the custom result listener animation with the Glide pop animation --- .../ui/notifications/blocks/NoteBlock.java | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java b/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java index 570821795d0c..7c453730f896 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java @@ -9,12 +9,9 @@ import android.view.Gravity; import android.view.View; import android.view.ViewGroup; -import android.view.animation.Animation; -import android.view.animation.AnimationUtils; import android.widget.Button; import android.widget.FrameLayout; import android.widget.ImageView; -import android.widget.ImageView.ScaleType; import android.widget.LinearLayout; import android.widget.MediaController; import android.widget.VideoView; @@ -26,6 +23,7 @@ import org.wordpress.android.fluxc.tools.FormattableContent; import org.wordpress.android.fluxc.tools.FormattableMedia; import org.wordpress.android.fluxc.tools.FormattableRange; +import org.wordpress.android.util.image.GlidePopTransitionOptions; import org.wordpress.android.ui.notifications.utils.NotificationsUtilsWrapper; import org.wordpress.android.util.AccessibilityUtils; import org.wordpress.android.util.AppLog; @@ -48,7 +46,6 @@ public class NoteBlock { protected final NotificationsUtilsWrapper mNotificationsUtilsWrapper; private boolean mIsBadge; private boolean mIsPingback; - private boolean mHasAnimatedBadge; private boolean mIsViewMilestone; public interface OnNoteBlockTextClickListener { @@ -161,27 +158,22 @@ public View configureView(final View view) { if (hasImageMediaItem()) { noteBlockHolder.getImageView().setVisibility(View.VISIBLE); // Request image, and animate it when loaded - mImageManager - .loadWithResultListener(noteBlockHolder.getImageView(), ImageType.IMAGE, - StringUtils.notNullStr(getNoteMediaItem().getUrl()), ScaleType.CENTER, null, - new ImageManager.RequestListener() { - @Override - public void onLoadFailed(@Nullable Exception e, @Nullable Object model) { - if (e != null) { - AppLog.e(T.NOTIFS, e); - } - noteBlockHolder.hideImageView(); - } + mImageManager.animateWithResultListener(noteBlockHolder.getImageView(), ImageType.IMAGE, + StringUtils.notNullStr(getNoteMediaItem().getUrl()), + GlidePopTransitionOptions.INSTANCE.pop(), + new ImageManager.RequestListener() { + @Override + public void onLoadFailed(@Nullable Exception e, @Nullable Object model) { + if (e != null) { + AppLog.e(T.NOTIFS, e); + } + noteBlockHolder.hideImageView(); + } - @Override - public void onResourceReady(@NonNull Drawable resource, @Nullable Object model) { - if (!mHasAnimatedBadge && view.getContext() != null) { - mHasAnimatedBadge = true; - Animation pop = AnimationUtils.loadAnimation(view.getContext(), R.anim.pop); - noteBlockHolder.getImageView().startAnimation(pop); - } - } - }); + @Override + public void onResourceReady(@NonNull Drawable resource, @Nullable Object model) { + } + }); if (mIsBadge) { noteBlockHolder.getImageView().setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_NO); From f4ba3e6e6137325ff8f43629a7f5dfc3d89f93fb Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Mar 2024 17:31:40 +0200 Subject: [PATCH 3/8] Adds tests --- .../image/GlidePopTransitionOptionsTest.kt | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 WordPress/src/test/java/org/wordpress/android/util/image/GlidePopTransitionOptionsTest.kt diff --git a/WordPress/src/test/java/org/wordpress/android/util/image/GlidePopTransitionOptionsTest.kt b/WordPress/src/test/java/org/wordpress/android/util/image/GlidePopTransitionOptionsTest.kt new file mode 100644 index 000000000000..02b15ec498c9 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/util/image/GlidePopTransitionOptionsTest.kt @@ -0,0 +1,49 @@ +package org.wordpress.android.util.image + +import android.graphics.drawable.Drawable +import android.view.View +import com.bumptech.glide.load.DataSource +import com.bumptech.glide.request.transition.NoTransition +import com.bumptech.glide.request.transition.Transition +import junit.framework.TestCase +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.junit.Test +import org.mockito.Mockito +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest + +@ExperimentalCoroutinesApi +class GlidePopTransitionOptionsTest : BaseUnitTest() { + @Test + fun testBuildWithCache() { + val glidePopTransitionFactory = GlidePopTransitionFactory() + + val result = glidePopTransitionFactory.build(DataSource.MEMORY_CACHE, true) + + TestCase.assertTrue(result is NoTransition) + } + + @Test + fun testBuildWithNoCache() { + val glidePopTransitionFactory = GlidePopTransitionFactory() + + val result = glidePopTransitionFactory.build(DataSource.REMOTE, true) + + TestCase.assertTrue(result is GlidePopTransition) + } + + @Test + fun testTransition() { + val glidePopTransition = GlidePopTransition() + val drawable: Drawable = Mockito.mock() + val viewAdapter: Transition.ViewAdapter = Mockito.mock() + val view: View = Mockito.mock() + whenever(viewAdapter.view).thenReturn(view) + whenever(view.context).thenReturn(Mockito.mock()) + + val result = glidePopTransition.transition(drawable, viewAdapter) + + TestCase.assertTrue(result) + Mockito.verify(viewAdapter).setDrawable(drawable) + } +} From e9d0903bf4c2fa2ceb6bae39ca444c98ba0ef26b Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Mar 2024 17:31:58 +0200 Subject: [PATCH 4/8] Adds release note --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index efeec6fb0f33..6e19b5ac76e4 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -5,6 +5,7 @@ * [*] [internal] Block editor: Remove code associated to Story block [https://github.com/wordpress-mobile/WordPress-Android/pull/20400] * [*] [Jetpack-only] Fixes broken links on some notifications [https://github.com/wordpress-mobile/WordPress-Android/pull/20417] * [**] [internal] Block editor: Upgrade React Native to version 0.73.3 [#20167] +* [*] [Jetpack-only] Fixes notification badges loading error [https://github.com/wordpress-mobile/WordPress-Android/pull/20431] 24.4 ----- From 44b79ababe0d4ff2511af816cba6bb4186e7777e Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 12 Mar 2024 12:34:57 +0200 Subject: [PATCH 5/8] Replaces custom image view initialisation with layout initialisation --- .../android/ui/notifications/blocks/NoteBlock.java | 9 +-------- WordPress/src/main/res/layout/note_block_basic.xml | 9 +++++++++ WordPress/src/main/res/values/dimens.xml | 1 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java b/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java index 7c453730f896..ef18f36c757d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java @@ -310,14 +310,7 @@ public View getDivider() { public ImageView getImageView() { if (mImageView == null) { - mImageView = new ImageView(mRootLayout.getContext()); - int imageSize = DisplayUtils.dpToPx(mRootLayout.getContext(), 180); - int imagePadding = mRootLayout.getContext().getResources().getDimensionPixelSize(R.dimen.margin_large); - LinearLayout.LayoutParams layoutParams = new LinearLayout.LayoutParams(imageSize, imageSize); - layoutParams.gravity = Gravity.CENTER_HORIZONTAL; - mImageView.setLayoutParams(layoutParams); - mImageView.setPadding(0, imagePadding, 0, 0); - mRootLayout.addView(mImageView, 0); + mImageView = mRootLayout.findViewById(R.id.image); } return mImageView; diff --git a/WordPress/src/main/res/layout/note_block_basic.xml b/WordPress/src/main/res/layout/note_block_basic.xml index aa4c92bf26f7..448634c3962a 100644 --- a/WordPress/src/main/res/layout/note_block_basic.xml +++ b/WordPress/src/main/res/layout/note_block_basic.xml @@ -6,6 +6,15 @@ android:layout_gravity="center" android:orientation="vertical"> + + 40dp 44dp 48dp + 180dp 3dp 5dp From 8094310da11046ce30446dc622b43c9889b24931 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 14 Mar 2024 15:52:29 +0200 Subject: [PATCH 6/8] Revert "Adds release note" This reverts commit e9d0903bf4c2fa2ceb6bae39ca444c98ba0ef26b. --- RELEASE-NOTES.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 6fbb416af946..5a2791307788 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -5,7 +5,6 @@ * [*] [internal] Block editor: Remove code associated to Story block [https://github.com/wordpress-mobile/WordPress-Android/pull/20400] * [*] [Jetpack-only] Fixes broken links on some notifications [https://github.com/wordpress-mobile/WordPress-Android/pull/20417] * [**] [internal] Block editor: Upgrade React Native to version 0.73.3 [#20167] -* [*] [Jetpack-only] Fixes notification badges loading error [https://github.com/wordpress-mobile/WordPress-Android/pull/20431] 24.4 ----- From ab029691d5fab6b72b4bdc58538ecb118a4e43a5 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 14 Mar 2024 16:36:49 +0200 Subject: [PATCH 7/8] Preloads notification image --- .../NotificationsDetailListFragment.kt | 5 +++++ .../ui/notifications/blocks/NoteBlock.java | 4 ++-- .../android/util/image/ImageManager.kt | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt index 27daa1969976..1addf88cffc1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt @@ -422,6 +422,11 @@ class NotificationsDetailListFragment : ListFragment(), NotificationFragment { noteObject, imageManager, notificationsUtilsWrapper, mOnNoteBlockTextClickListener ) + if (noteBlock.hasImageMediaItem()) { + noteBlock.noteMediaItem?.url?.let { + imageManager.preload(requireContext(), it) + } + } } // Badge notifications apply different colors and formatting diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java b/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java index ef18f36c757d..ae7bb9d88764 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/blocks/NoteBlock.java @@ -104,7 +104,7 @@ public void setIsPingback() { mIsPingback = true; } - FormattableMedia getNoteMediaItem() { + @Nullable public FormattableMedia getNoteMediaItem() { return FormattableContentUtilsKt.getMediaOrNull(mNoteData, 0); } @@ -124,7 +124,7 @@ private boolean hasMediaArray() { return mNoteData.getMedia() != null && !mNoteData.getMedia().isEmpty(); } - boolean hasImageMediaItem() { + public boolean hasImageMediaItem() { return hasMediaArray() && getNoteMediaItem() != null && !TextUtils.isEmpty(getNoteMediaItem().getType()) diff --git a/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt b/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt index 591bb748cf83..11d5fcb7dd1f 100644 --- a/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt +++ b/WordPress/src/main/java/org/wordpress/android/util/image/ImageManager.kt @@ -326,6 +326,23 @@ class ImageManager @Inject constructor( .clearOnDetach() } + /** + * Preloads an image from the provided `imgUrl`. + */ + fun preload(context: Context, imgUrl: String) { + if (!context.isAvailable()) return + try { + Glide.with(context) + .downloadOnly() + .load(Uri.parse(imgUrl)) + .submit() + .get() // This makes each call blocking, so subsequent calls can be cancelled if needed. + } catch (e: ExecutionException) { + // This is a best effort preload, so we don't want to crash the app if an `ExecutionException` is thrown. + AppLog.e(AppLog.T.UTILS, "Error preloading image $imgUrl: $e") + } + } + /** * Preloads an [MShot]. * From 8a8e1c1c6ddd1e04a4a1ac08e645cbc3ad876317 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 14 Mar 2024 16:50:04 +0200 Subject: [PATCH 8/8] Fixes detekt issue --- .../NotificationsDetailListFragment.kt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt index 1addf88cffc1..667c7ada8944 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsDetailListFragment.kt @@ -422,11 +422,7 @@ class NotificationsDetailListFragment : ListFragment(), NotificationFragment { noteObject, imageManager, notificationsUtilsWrapper, mOnNoteBlockTextClickListener ) - if (noteBlock.hasImageMediaItem()) { - noteBlock.noteMediaItem?.url?.let { - imageManager.preload(requireContext(), it) - } - } + preloadImage(noteBlock) } // Badge notifications apply different colors and formatting @@ -452,6 +448,14 @@ class NotificationsDetailListFragment : ListFragment(), NotificationFragment { return pingbackUrl } + private fun preloadImage(noteBlock: NoteBlock) { + if (noteBlock.hasImageMediaItem()) { + noteBlock.noteMediaItem?.url?.let { + imageManager.preload(requireContext(), it) + } + } + } + @Suppress("OVERRIDE_DEPRECATION") override fun doInBackground(vararg params: Void): List? { if (notification == null) {