From 5690523c6dc40c435c3d81868d89ba26d21e3663 Mon Sep 17 00:00:00 2001 From: Jared Burrows Date: Sat, 6 Aug 2022 21:00:06 -0400 Subject: [PATCH] migrate item in recyclerview to compose (#211) --- app/build.gradle.kts | 1 + .../example/gif/ui/giflist/GifActivityTest.kt | 52 ++++----- .../gif/ui/license/LicenseActivityTest.kt | 27 ++--- .../example/gif/data/ImageService.kt | 31 ++++++ .../example/gif/ui/giflist/GifActivity.kt | 10 +- .../example/gif/ui/giflist/GifAdapter.kt | 103 ++++++++++-------- app/src/main/res/layout/activity_gif.xml | 4 +- app/src/main/res/layout/list_item.xml | 32 ------ .../data/source/network/DtoConstructorTest.kt | 4 +- .../example/gif/ui/giflist/GifAdapterTest.kt | 2 +- gradle/libs.versions.toml | 1 + 11 files changed, 136 insertions(+), 131 deletions(-) delete mode 100644 app/src/main/res/layout/list_item.xml diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 0ca5d604..2a71f450 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -219,6 +219,7 @@ dependencies { implementation(libs.androidx.compose.runtime) implementation(libs.androidx.compose.ui) implementation(libs.androidx.compose.uitooling) + implementation(libs.google.accompanist.drawablepainter) implementation(libs.google.accompanist.webview) androidTestImplementation(libs.androidx.compose.junit) diff --git a/app/src/androidTest/java/com/burrowsapps/example/gif/ui/giflist/GifActivityTest.kt b/app/src/androidTest/java/com/burrowsapps/example/gif/ui/giflist/GifActivityTest.kt index f19c0681..80244a3d 100644 --- a/app/src/androidTest/java/com/burrowsapps/example/gif/ui/giflist/GifActivityTest.kt +++ b/app/src/androidTest/java/com/burrowsapps/example/gif/ui/giflist/GifActivityTest.kt @@ -3,6 +3,7 @@ package com.burrowsapps.example.gif.ui.giflist import android.Manifest.permission.INTERNET import android.Manifest.permission.READ_EXTERNAL_STORAGE import android.Manifest.permission.WRITE_EXTERNAL_STORAGE +import android.content.Context import android.widget.TextView import androidx.arch.core.executor.testing.InstantTaskExecutorRule import androidx.compose.ui.test.junit4.createAndroidComposeRule @@ -14,12 +15,10 @@ import androidx.test.espresso.action.ViewActions.closeSoftKeyboard import androidx.test.espresso.action.ViewActions.pressBack import androidx.test.espresso.action.ViewActions.typeText import androidx.test.espresso.assertion.ViewAssertions.matches -import androidx.test.espresso.contrib.RecyclerViewActions.actionOnItem import androidx.test.espresso.intent.Intents.init import androidx.test.espresso.intent.Intents.intended import androidx.test.espresso.intent.Intents.release import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent -import androidx.test.espresso.matcher.ViewMatchers.hasDescendant import androidx.test.espresso.matcher.ViewMatchers.isDisplayed import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withParent @@ -32,6 +31,7 @@ import com.burrowsapps.example.gif.test.TestFileUtils.MOCK_SERVER_PORT import com.burrowsapps.example.gif.test.TestFileUtils.getMockFileResponse import com.burrowsapps.example.gif.test.TestFileUtils.getMockResponse import com.burrowsapps.example.gif.ui.license.LicenseActivity +import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltAndroidTest import dagger.hilt.android.testing.HiltTestApplication @@ -44,13 +44,13 @@ import org.hamcrest.Matchers.containsString import org.hamcrest.Matchers.instanceOf import org.junit.After import org.junit.Before -import org.junit.Ignore import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.robolectric.annotation.Config import test.ScreenshotWatcher import java.net.HttpURLConnection.HTTP_NOT_FOUND +import javax.inject.Inject @HiltAndroidTest @Config(application = HiltTestApplication::class) @@ -72,6 +72,8 @@ class GifActivityTest { @get:Rule(order = 4) val screenshotWatcher = ScreenshotWatcher() + @Inject @ApplicationContext internal lateinit var context: Context + private val server = MockWebServer() @Before @@ -102,13 +104,13 @@ class GifActivityTest { } @Test - fun testMainTitleIsShowing() { + fun testGifActivityTitleIsShowing() { onView( allOf( instanceOf(TextView::class.java), - withParent(withId(R.id.toolbar)) + withParent(withId(R.id.toolbar)), ) - ).check(matches(withText(containsString("Top Trending Gifs")))) + ).check(matches(withText(containsString(context.getString(R.string.main_screen_title))))) } @Test @@ -145,31 +147,31 @@ class GifActivityTest { onView( allOf( instanceOf(TextView::class.java), - withParent(withId(R.id.toolbar)) + withParent(withId(R.id.toolbar)), ) ).check(matches(withText(containsString("Top Trending Gifs")))) release() } - @Ignore("on view 'Animations or transitions are enabled on the target device.") - @Test - fun testTrendingThenClickOpenDialog() { - screenshotWatcher.capture("After launch") - - // Select 0, the response only contains 1 item - onView(withId(R.id.recyclerView)) - .perform( - actionOnItem( - hasDescendant(withId(R.id.gifImage)), - click() - ).atPosition(0) - ) - screenshotWatcher.capture("After click") - - onView(withId(R.id.gifDialogTitle)) - .perform(pressBack()) - } +// @Ignore("on view 'Animations or transitions are enabled on the target device.") +// @Test +// fun testTrendingThenClickOpenDialog() { +// screenshotWatcher.capture("After launch") +// +// // Select 0, the response only contains 1 item +// onView(withId(R.id.recyclerView)) +// .perform( +// actionOnItem( +// hasDescendant(withId(R.id.gifImage)), +// click(), +// ).atPosition(0) +// ) +// screenshotWatcher.capture("After click") +// +// onView(withId(R.id.gifDialogTitle)) +// .perform(pressBack()) +// } @Test fun testTrendingResultsThenSearchThenBackToTrending() { diff --git a/app/src/androidTest/java/com/burrowsapps/example/gif/ui/license/LicenseActivityTest.kt b/app/src/androidTest/java/com/burrowsapps/example/gif/ui/license/LicenseActivityTest.kt index b98bc60a..fda21ea0 100644 --- a/app/src/androidTest/java/com/burrowsapps/example/gif/ui/license/LicenseActivityTest.kt +++ b/app/src/androidTest/java/com/burrowsapps/example/gif/ui/license/LicenseActivityTest.kt @@ -3,28 +3,25 @@ package com.burrowsapps.example.gif.ui.license import android.Manifest.permission.INTERNET import android.Manifest.permission.READ_EXTERNAL_STORAGE import android.Manifest.permission.WRITE_EXTERNAL_STORAGE -import android.widget.TextView -import androidx.appcompat.widget.Toolbar +import android.content.Context import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.junit4.createAndroidComposeRule -import androidx.test.espresso.Espresso.onView -import androidx.test.espresso.assertion.ViewAssertions.matches -import androidx.test.espresso.matcher.ViewMatchers.withParent -import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.compose.ui.test.onNodeWithText import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.rule.GrantPermissionRule +import com.burrowsapps.example.gif.R +import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltAndroidTest import dagger.hilt.android.testing.HiltTestApplication -import org.hamcrest.Matchers.allOf -import org.hamcrest.Matchers.containsString -import org.hamcrest.Matchers.instanceOf import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.robolectric.annotation.Config import test.ScreenshotWatcher +import javax.inject.Inject @HiltAndroidTest @Config(application = HiltTestApplication::class) @@ -46,18 +43,16 @@ class LicenseActivityTest { @get:Rule(order = 4) val screenshotWatcher = ScreenshotWatcher() + @Inject @ApplicationContext internal lateinit var context: Context + @Before fun setUp() { hiltRule.inject() } @Test - fun testLicensesTitleIsShowing() { - onView( - allOf( - instanceOf(TextView::class.java), - withParent(instanceOf(Toolbar::class.java)) - ) - ).check(matches(withText(containsString("Open source licenses")))) + fun testLicensesActivityTitleIsShowing() { + composeTestRule.onNodeWithText(context.getString(R.string.menu_licenses)) + .assertIsDisplayed() } } diff --git a/app/src/main/java/com/burrowsapps/example/gif/data/ImageService.kt b/app/src/main/java/com/burrowsapps/example/gif/data/ImageService.kt index 37dc4023..7d73e66e 100644 --- a/app/src/main/java/com/burrowsapps/example/gif/data/ImageService.kt +++ b/app/src/main/java/com/burrowsapps/example/gif/data/ImageService.kt @@ -1,6 +1,7 @@ package com.burrowsapps.example.gif.data import android.content.Context +import android.graphics.drawable.Drawable import android.widget.ImageView import com.bumptech.glide.RequestBuilder import com.bumptech.glide.load.DataSource @@ -8,14 +9,44 @@ import com.bumptech.glide.load.engine.GlideException import com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions.withCrossFade import com.bumptech.glide.load.resource.gif.GifDrawable import com.bumptech.glide.request.RequestListener +import com.bumptech.glide.request.target.CustomTarget import com.bumptech.glide.request.target.Target import com.bumptech.glide.request.target.Target.SIZE_ORIGINAL +import com.bumptech.glide.request.transition.Transition import com.burrowsapps.example.gif.di.GlideApp import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject class ImageService @Inject constructor(@ApplicationContext private val context: Context) { + fun loadGif( + imageUrl: String, + thumbnailUrl: String, + onResourceReady: (GifDrawable?) -> Unit, + onLoadFailed: () -> Unit, + ) { + loadGif(imageUrl) + .override(SIZE_ORIGINAL, SIZE_ORIGINAL) + .thumbnail(loadGif(thumbnailUrl)) + .into(object : CustomTarget() { + override fun onLoadFailed(errorDrawable: Drawable?) { + super.onLoadFailed(errorDrawable) + onLoadFailed.invoke() + } + + override fun onLoadCleared(placeholder: Drawable?) { + onLoadFailed.invoke() + } + + override fun onResourceReady( + resource: GifDrawable, + transition: Transition?, + ) { + onResourceReady.invoke(resource) + } + }) + } + fun loadGif( imageUrl: String, thumbnailUrl: String, diff --git a/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifActivity.kt b/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifActivity.kt index 725849ec..a768c945 100644 --- a/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifActivity.kt +++ b/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifActivity.kt @@ -64,9 +64,9 @@ class GifActivity : AppCompatActivity() { (1.0 * Resources.getSystem().displayMetrics.density).roundToInt(), // TODO 1.dp in compose gridLayoutManager.spanCount ) - gifAdapter = GifAdapter(onItemClick = { imageInfoModel -> + gifAdapter = GifAdapter(imageService) { imageInfoModel -> showImageDialog(imageInfoModel) - }, imageService) + } // Setup RecyclerView activityBinding.recyclerView.apply { @@ -76,12 +76,6 @@ class GifActivity : AppCompatActivity() { setHasFixedSize(true) setItemViewCacheSize(DEFAULT_LIMIT_COUNT) // default 2 recycledViewPool.setMaxRecycledViews(0, PORTRAIT_COLUMNS * 2) // default 5 - addRecyclerListener { holder -> - val gifViewHolder = holder as GifAdapter.ViewHolder - GlideApp.with(this).clear(gifViewHolder.listBinding.gifImage) - - Timber.i("addRecyclerListener") - } addOnScrollListener( object : RecyclerView.OnScrollListener() { override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) { diff --git a/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifAdapter.kt b/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifAdapter.kt index 391aa054..8696aff0 100644 --- a/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifAdapter.kt +++ b/app/src/main/java/com/burrowsapps/example/gif/ui/giflist/GifAdapter.kt @@ -1,19 +1,31 @@ package com.burrowsapps.example.gif.ui.giflist -import android.view.LayoutInflater import android.view.ViewGroup +import androidx.compose.foundation.Image +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.platform.ComposeView +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp import androidx.recyclerview.widget.RecyclerView +import com.bumptech.glide.load.resource.gif.GifDrawable +import com.burrowsapps.example.gif.R import com.burrowsapps.example.gif.data.ImageService -import com.burrowsapps.example.gif.databinding.ListItemBinding -import com.burrowsapps.example.gif.di.GlideApp -import timber.log.Timber +import com.burrowsapps.example.gif.ui.theme.GifTheme +import com.google.accompanist.drawablepainter.rememberDrawablePainter /** * RecyclerView adapter for handling Gif Images in a Grid format. */ class GifAdapter( - private var onItemClick: (GifImageInfo) -> Unit, private val imageService: ImageService, + private var onItemClick: (GifImageInfo) -> Unit, ) : RecyclerView.Adapter() { private val data = mutableListOf() @@ -24,7 +36,7 @@ class GifAdapter( override fun onCreateViewHolder( parent: ViewGroup, viewType: Int - ) = ViewHolder(ListItemBinding.inflate(LayoutInflater.from(parent.context), parent, false)) + ) = ViewHolder(ComposeView(parent.context)) override fun onBindViewHolder(holder: ViewHolder, position: Int) { val imageInfoModel = getItem(position) @@ -32,25 +44,6 @@ class GifAdapter( holder.bind(imageInfoModel) } - override fun onViewRecycled(holder: ViewHolder) { - super.onViewRecycled(holder) - - // https://github.com/bumptech/glide/issues/624#issuecomment-140134792 - // Forget view, try to free resources - GlideApp.with(holder.itemView.context).clear(holder.listBinding.gifImage) - holder.listBinding.apply { - gifImage.setImageDrawable(null) - // Make sure to show progress when loading new view - gifProgress.show() - } - Timber.i("onViewRecycled") - } - - override fun onFailedToRecycleView(holder: ViewHolder): Boolean { - Timber.e("onFailedToRecycleView") - return false - } - override fun getItemCount() = data.size override fun getItemId(position: Int) = getItem(position).hashCode().toLong() @@ -72,28 +65,50 @@ class GifAdapter( } inner class ViewHolder( - val listBinding: ListItemBinding, - ) : RecyclerView.ViewHolder(listBinding.root) { - + private val composeView: ComposeView, + ) : RecyclerView.ViewHolder(composeView) { fun bind(imageInfoModel: GifImageInfo) { itemView.setOnClickListener { onItemClick.invoke(imageInfoModel) } - // Load images - 'tinyGifPreviewUrl' -> 'tinyGifUrl' - imageService.loadGif( - imageUrl = imageInfoModel.tinyGifUrl, - thumbnailUrl = imageInfoModel.tinyGifPreviewUrl, - imageView = listBinding.gifImage, - onResourceReady = { - // Hide progressbar - listBinding.gifProgress.hide() - Timber.i("onResourceReady") - }, - onLoadFailed = { e -> - // Hide progressbar - listBinding.gifProgress.hide() - Timber.e(e, "onLoadFailed") - }, - ) + composeView.setContent { + val showProgressBar = remember { mutableStateOf(true) } + val state = remember { mutableStateOf(null) } + + GifTheme { + // Load images - 'tinyGifPreviewUrl' -> 'tinyGifUrl' + imageService.loadGif( + imageUrl = imageInfoModel.tinyGifUrl, + thumbnailUrl = imageInfoModel.tinyGifPreviewUrl, + onResourceReady = { resource -> + showProgressBar.value = false + state.value = resource + }, + onLoadFailed = { + showProgressBar.value = false + state.value = null + }, + ) + + // Show loading indicator when image is not loaded + if (showProgressBar.value) { + CircularProgressIndicator( + modifier = Modifier + .fillMaxWidth() + .height(128.dp) + .padding(all = 24.dp), + ) + } else { + Image( + painter = rememberDrawablePainter(drawable = state.value), + contentDescription = stringResource(id = R.string.gif_image), + contentScale = ContentScale.Crop, + modifier = Modifier + .fillMaxWidth() + .height(135.dp), + ) + } + } + } } } } diff --git a/app/src/main/res/layout/activity_gif.xml b/app/src/main/res/layout/activity_gif.xml index b3b2ed42..297e3cc9 100644 --- a/app/src/main/res/layout/activity_gif.xml +++ b/app/src/main/res/layout/activity_gif.xml @@ -2,7 +2,6 @@ @@ -30,7 +29,6 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:clipToPadding="false" - android:scrollbars="none" - tools:listitem="@layout/list_item" /> + android:scrollbars="none" /> diff --git a/app/src/main/res/layout/list_item.xml b/app/src/main/res/layout/list_item.xml deleted file mode 100644 index 22ce657f..00000000 --- a/app/src/main/res/layout/list_item.xml +++ /dev/null @@ -1,32 +0,0 @@ - - - - - - - diff --git a/app/src/test/java/com/burrowsapps/example/gif/data/source/network/DtoConstructorTest.kt b/app/src/test/java/com/burrowsapps/example/gif/data/source/network/DtoConstructorTest.kt index 1e5321b4..597966a7 100644 --- a/app/src/test/java/com/burrowsapps/example/gif/data/source/network/DtoConstructorTest.kt +++ b/app/src/test/java/com/burrowsapps/example/gif/data/source/network/DtoConstructorTest.kt @@ -17,7 +17,7 @@ import java.lang.reflect.Modifier @RunWith(AndroidJUnit4::class) class DtoConstructorTest { - private val modelPackage = "burrows.apps.example.gif.data.rest.model" + private val modelPackage = "com.burrowsapps.example.gif.data.source.network" private val dataTransportObject = "Dto" private lateinit var classes: Set> @@ -34,7 +34,7 @@ class DtoConstructorTest { Scanners.MethodsAnnotated, Scanners.MethodsParameter, MethodParameterNamesScanner(), - MemberUsageScanner() + MemberUsageScanner(), ) ) classes = reflections.getSubTypesOf(Any::class.java) diff --git a/app/src/test/java/com/burrowsapps/example/gif/ui/giflist/GifAdapterTest.kt b/app/src/test/java/com/burrowsapps/example/gif/ui/giflist/GifAdapterTest.kt index ed75a1bf..85a2f364 100644 --- a/app/src/test/java/com/burrowsapps/example/gif/ui/giflist/GifAdapterTest.kt +++ b/app/src/test/java/com/burrowsapps/example/gif/ui/giflist/GifAdapterTest.kt @@ -38,9 +38,9 @@ class GifAdapterTest { hiltRule.inject() sut = GifAdapter( + imageService, onItemClick = { }, - imageService ).apply { add(listOf(gifImageInfo, gifImageInfo2)) } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 58e9b501..57e2a991 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -81,6 +81,7 @@ bumptech-glide = { module = "com.github.bumptech.glide:glide", version.ref = "gl bumptech-glide-compiler = { module = "com.github.bumptech.glide:compiler", version.ref = "glide" } bumptech-glide-okhttp = { module = "com.github.bumptech.glide:okhttp3-integration", version.ref = "glide" } glassfish-javax-annotation = { module = "org.glassfish:javax.annotation", version = "10.0-b28" } +google-accompanist-drawablepainter = { module = "com.google.accompanist:accompanist-drawablepainter", version = "0.26.0-alpha" } google-accompanist-webview = { module = "com.google.accompanist:accompanist-webview", version = "0.26.0-alpha" } google-findbugs-jsr305 = { module = "com.google.code.findbugs:jsr305", version = "3.0.2" } google-hilt-android = { module = "com.google.dagger:hilt-android", version.ref = "dagger" }