Skip to content

Commit

Permalink
Fix #2132: Chapter List Page Issues (#2232) - Blur and lock thumbnail…
Browse files Browse the repository at this point in the history
…s of inactive cards. (#2422)

* Lock and blur

* Replace color/white with #FFFFFF

* Add \n

* Move isBlurred to LessonThumbnailImageView.

* Remove unused import.

* Move dependency on Glide away from from ImageLoader.

* Remove unneeded line.

* Refactor Transformation-related code, add KDocs.

* Renaming.

* Remove unused import.

* Reformat.

* Add Mockk test

* Add Mockk test - lint

* Add blurring tests.

* Mock ImageLoader in StoryActivityTest.

* Add delay before image loading.

* Nits and improvements.

* Remove unused import.

* Remove unused import.

* Set image resources in LessonThumbnailImageView in addition to calling GlideImageLoader.

* BlurTransformation re-write.

* BlurTransformation re-write.

* Add TestImageLoaderModule to some tests.

* Lint

* Add TestImageLoaderModule to more tests.

* Add TestImageLoaderModule to more tests.

* Update utility/src/main/java/org/oppia/android/util/parser/BlurTransformation.kt

Co-authored-by: Ben Henning <[email protected]>

* Update utility/src/main/java/org/oppia/android/util/parser/BlurTransformation.kt

Co-authored-by: Ben Henning <[email protected]>

* TestGlideImageLoader Refactor.

* Reformat.

* Reformat.

* Reformat.

* Nits and comments.

* Revert changes in landscape design.

* Change BlurTransformation to return a copy instead of 'toTransform'.

* Update app/src/sharedTest/java/org/oppia/android/app/story/StoryFragmentTest.kt

Co-authored-by: Ben Henning <[email protected]>

* Refactor TestGlideImageLoader.

* Nits

Co-authored-by: Ben Henning <[email protected]>
  • Loading branch information
viktoriias and BenHenning authored Feb 3, 2021
1 parent 9524453 commit 2bcd511
Show file tree
Hide file tree
Showing 15 changed files with 356 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import org.oppia.android.util.gcsresource.DefaultResourceBucketName
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.parser.DefaultGcsPrefix
import org.oppia.android.util.parser.ImageLoader
import org.oppia.android.util.parser.ImageTransformation
import org.oppia.android.util.parser.ImageViewTarget
import org.oppia.android.util.parser.ThumbnailDownloadUrlTemplate
import javax.inject.Inject
Expand All @@ -25,6 +26,7 @@ class LessonThumbnailImageView @JvmOverloads constructor(
) : AppCompatImageView(context, attrs, defStyleAttr) {

private val imageView = this
private var isBlurred: Boolean = false
private lateinit var lessonThumbnail: LessonThumbnail
private lateinit var entityId: String
private lateinit var entityType: String
Expand Down Expand Up @@ -63,6 +65,10 @@ class LessonThumbnailImageView @JvmOverloads constructor(
checkIfLoadingIsPossible()
}

fun setIsBlurred(isBlurred: Boolean) {
this.isBlurred = isBlurred
}

private fun checkIfLoadingIsPossible() {
if (::entityId.isInitialized &&
::entityType.isInitialized &&
Expand All @@ -78,16 +84,25 @@ class LessonThumbnailImageView @JvmOverloads constructor(
}

private fun loadLessonThumbnail() {
var transformations = if (isBlurred) {
listOf(ImageTransformation.BLUR)
} else {
listOf()
}
if (lessonThumbnail.thumbnailFilename.isNotEmpty()) {
loadImage(lessonThumbnail.thumbnailFilename)
loadImage(lessonThumbnail.thumbnailFilename, transformations)
} else {
imageView.setImageResource(getLessonDrawableResource(lessonThumbnail))
imageLoader.loadDrawable(
getLessonDrawableResource(lessonThumbnail),
ImageViewTarget(this),
transformations
)
}
imageView.setBackgroundColor(lessonThumbnail.backgroundColorRgb)
}

/** Loads an image using Glide from [filename]. */
private fun loadImage(filename: String) {
private fun loadImage(filename: String, transformations: List<ImageTransformation>) {
val imageName = String.format(
thumbnailDownloadUrlTemplate,
entityType,
Expand All @@ -96,9 +111,9 @@ class LessonThumbnailImageView @JvmOverloads constructor(
)
val imageUrl = "$gcsPrefix/$resourceBucketName/$imageName"
if (imageUrl.endsWith("svg", ignoreCase = true)) {
imageLoader.loadSvg(imageUrl, ImageViewTarget(this))
imageLoader.loadSvg(imageUrl, ImageViewTarget(this), transformations)
} else {
imageLoader.loadBitmap(imageUrl, ImageViewTarget(this))
imageLoader.loadBitmap(imageUrl, ImageViewTarget(this), transformations)
}
}

Expand Down
10 changes: 10 additions & 0 deletions app/src/main/res/drawable/ic_baseline_lock_24.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="80dp"
android:height="80dp"
android:tint="#FFFFFF"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:fillColor="#FFFFFF"
android:pathData="M18,8h-1L17,6c0,-2.76 -2.24,-5 -5,-5S7,3.24 7,6v2L6,8c-1.1,0 -2,0.9 -2,2v10c0,1.1 0.9,2 2,2h12c1.1,0 2,-0.9 2,-2L20,10c0,-1.1 -0.9,-2 -2,-2zM12,17c-1.1,0 -2,-0.9 -2,-2s0.9,-2 2,-2 2,0.9 2,2 -0.9,2 -2,2zM15.1,8L8.9,8L8.9,6c0,-1.71 1.39,-3.1 3.1,-3.1 1.71,0 3.1,1.39 3.1,3.1v2z" />
</vector>
23 changes: 23 additions & 0 deletions app/src/main/res/layout-sw600dp/story_chapter_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,35 @@
android:scaleType="centerInside"
app:entityId="@{viewModel.storyId}"
app:entityType="@{viewModel.entityType}"
app:isBlurred="@{viewModel.chapterSummary.chapterPlayState == ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES}"
app:layout_constraintDimensionRatio="16:9"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:lessonThumbnail="@{viewModel.chapterThumbnail}" />

<View
android:layout_width="match_parent"
android:layout_height="0dp"
android:alpha="0.5"
android:background="@{viewModel.chapterThumbnail.backgroundColorRgb}"
android:visibility="@{viewModel.chapterSummary.chapterPlayState == ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.VISIBLE : View.INVISIBLE}"
app:layout_constraintBottom_toBottomOf="@id/chapter_thumbnail"
app:layout_constraintEnd_toEndOf="@id/chapter_thumbnail"
app:layout_constraintStart_toStartOf="@id/chapter_thumbnail"
app:layout_constraintTop_toTopOf="@id/chapter_thumbnail" />

<ImageView
android:layout_width="match_parent"
android:layout_height="match_parent"
android:scaleType="center"
android:src="@drawable/ic_baseline_lock_24"
android:visibility="@{viewModel.chapterSummary.chapterPlayState == ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.VISIBLE : View.INVISIBLE}"
app:layout_constraintBottom_toBottomOf="@id/chapter_thumbnail"
app:layout_constraintEnd_toEndOf="@id/chapter_thumbnail"
app:layout_constraintStart_toStartOf="@id/chapter_thumbnail"
app:layout_constraintTop_toTopOf="@id/chapter_thumbnail" />

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
Expand Down
23 changes: 23 additions & 0 deletions app/src/main/res/layout/story_chapter_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,35 @@
android:scaleType="centerInside"
app:entityId="@{viewModel.storyId}"
app:entityType="@{viewModel.entityType}"
app:isBlurred="@{viewModel.chapterSummary.chapterPlayState == ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES}"
app:layout_constraintDimensionRatio="16:9"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:lessonThumbnail="@{viewModel.chapterThumbnail}" />

<View
android:layout_width="match_parent"
android:layout_height="0dp"
android:alpha="0.5"
android:background="@{viewModel.chapterThumbnail.backgroundColorRgb}"
android:visibility="@{viewModel.chapterSummary.chapterPlayState == ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.VISIBLE : View.INVISIBLE}"
app:layout_constraintBottom_toBottomOf="@id/chapter_thumbnail"
app:layout_constraintEnd_toEndOf="@id/chapter_thumbnail"
app:layout_constraintStart_toStartOf="@id/chapter_thumbnail"
app:layout_constraintTop_toTopOf="@id/chapter_thumbnail" />

<ImageView
android:layout_width="match_parent"
android:layout_height="match_parent"
android:scaleType="center"
android:src="@drawable/ic_baseline_lock_24"
android:visibility="@{viewModel.chapterSummary.chapterPlayState == ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.VISIBLE : View.INVISIBLE}"
app:layout_constraintBottom_toBottomOf="@id/chapter_thumbnail"
app:layout_constraintEnd_toEndOf="@id/chapter_thumbnail"
app:layout_constraintStart_toStartOf="@id/chapter_thumbnail"
app:layout_constraintTop_toTopOf="@id/chapter_thumbnail" />

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ import org.oppia.android.testing.RobolectricModule
import org.oppia.android.testing.TestAccessibilityModule
import org.oppia.android.testing.TestCoroutineDispatchers
import org.oppia.android.testing.TestDispatcherModule
import org.oppia.android.testing.TestImageLoaderModule
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.profile.ProfileTestHelper
import org.oppia.android.util.caching.testing.CachingTestModule
import org.oppia.android.util.gcsresource.GcsResourceModule
import org.oppia.android.util.logging.LoggerModule
import org.oppia.android.util.logging.firebase.FirebaseLogUploaderModule
import org.oppia.android.util.parser.GlideImageLoaderModule
import org.oppia.android.util.parser.HtmlParserEntityTypeModule
import org.oppia.android.util.parser.ImageParsingModule
import org.robolectric.annotation.Config
Expand Down Expand Up @@ -603,7 +603,7 @@ class RecentlyPlayedFragmentTest {
ItemSelectionInputModule::class, MultipleChoiceInputModule::class,
NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, TextInputRuleModule::class,
DragDropSortInputModule::class, ImageClickInputModule::class, InteractionsModule::class,
GcsResourceModule::class, GlideImageLoaderModule::class, ImageParsingModule::class,
GcsResourceModule::class, TestImageLoaderModule::class, ImageParsingModule::class,
HtmlParserEntityTypeModule::class, QuestionModule::class, TestLogReportingModule::class,
TestAccessibilityModule::class, LogStorageModule::class, CachingTestModule::class,
PrimeTopicAssetsControllerModule::class, ExpirationMetaDataRetrieverModule::class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ import org.oppia.android.testing.RobolectricModule
import org.oppia.android.testing.TestAccessibilityModule
import org.oppia.android.testing.TestCoroutineDispatchers
import org.oppia.android.testing.TestDispatcherModule
import org.oppia.android.testing.TestImageLoaderModule
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.util.caching.testing.CachingTestModule
import org.oppia.android.util.gcsresource.GcsResourceModule
import org.oppia.android.util.logging.LoggerModule
import org.oppia.android.util.logging.firebase.FirebaseLogUploaderModule
import org.oppia.android.util.parser.GlideImageLoaderModule
import org.oppia.android.util.parser.HtmlParserEntityTypeModule
import org.oppia.android.util.parser.ImageParsingModule
import org.robolectric.annotation.Config
Expand Down Expand Up @@ -158,7 +158,7 @@ class StoryActivityTest {
ItemSelectionInputModule::class, MultipleChoiceInputModule::class,
NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, TextInputRuleModule::class,
DragDropSortInputModule::class, ImageClickInputModule::class, InteractionsModule::class,
GcsResourceModule::class, GlideImageLoaderModule::class, ImageParsingModule::class,
GcsResourceModule::class, TestImageLoaderModule::class, ImageParsingModule::class,
HtmlParserEntityTypeModule::class, QuestionModule::class, TestLogReportingModule::class,
TestAccessibilityModule::class, LogStorageModule::class, CachingTestModule::class,
PrimeTopicAssetsControllerModule::class, ExpirationMetaDataRetrieverModule::class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,22 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.rule.ActivityTestRule
import com.google.common.truth.Truth.assertThat
import dagger.Component
import dagger.Module
import dagger.Provides
import org.hamcrest.CoreMatchers.allOf
import org.junit.After
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.Captor
import org.mockito.Mockito.atLeastOnce
import org.mockito.Mockito.mock
import org.mockito.Mockito.verify
import org.mockito.junit.MockitoJUnit
import org.mockito.junit.MockitoRule
import org.oppia.android.R
import org.oppia.android.app.activity.ActivityComponent
import org.oppia.android.app.application.ActivityComponentFactory
Expand All @@ -37,13 +47,16 @@ import org.oppia.android.app.application.ApplicationInjector
import org.oppia.android.app.application.ApplicationInjectorProvider
import org.oppia.android.app.application.ApplicationModule
import org.oppia.android.app.application.ApplicationStartupListenerModule
import org.oppia.android.app.customview.LessonThumbnailImageView
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.player.exploration.ExplorationActivity
import org.oppia.android.app.player.state.hintsandsolution.HintsAndSolutionConfigModule
import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPositionOnView
import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.hasItemCount
import org.oppia.android.app.shim.ViewBindingShimModule
import org.oppia.android.app.utility.OrientationChangeAction.Companion.orientationLandscape
import org.oppia.android.app.utility.anyOrNull
import org.oppia.android.app.utility.capture
import org.oppia.android.domain.classify.InteractionsModule
import org.oppia.android.domain.classify.rules.continueinteraction.ContinueModule
import org.oppia.android.domain.classify.rules.dragAndDropSortInput.DragDropSortInputModule
Expand Down Expand Up @@ -76,9 +89,10 @@ import org.oppia.android.util.caching.testing.CachingTestModule
import org.oppia.android.util.gcsresource.GcsResourceModule
import org.oppia.android.util.logging.LoggerModule
import org.oppia.android.util.logging.firebase.FirebaseLogUploaderModule
import org.oppia.android.util.parser.GlideImageLoaderModule
import org.oppia.android.util.parser.HtmlParserEntityTypeModule
import org.oppia.android.util.parser.ImageLoader
import org.oppia.android.util.parser.ImageParsingModule
import org.oppia.android.util.parser.ImageTransformation
import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
import javax.inject.Inject
Expand All @@ -89,6 +103,9 @@ import javax.inject.Singleton
@LooperMode(LooperMode.Mode.PAUSED)
@Config(application = StoryFragmentTest.TestApplication::class, qualifiers = "port-xxhdpi")
class StoryFragmentTest {
@Rule
@JvmField
val mockitoRule: MockitoRule = MockitoJUnit.rule()

@Inject
lateinit var profileTestHelper: ProfileTestHelper
Expand All @@ -102,6 +119,9 @@ class StoryFragmentTest {
@Inject
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers

@Captor
lateinit var listCaptor: ArgumentCaptor<List<ImageTransformation>>

@get:Rule
var activityTestRule: ActivityTestRule<StoryActivity> = ActivityTestRule(
StoryActivity::class.java, /* initialTouchMode= */ true, /* launchActivity= */ false
Expand Down Expand Up @@ -304,6 +324,65 @@ class StoryFragmentTest {
}
}

@Test
fun testStoryFragment_chapterMissingPrerequisiteThumbnailIsBlurred() {
launch<StoryActivity>(createFractionsStoryActivityIntent()).use {
testCoroutineDispatchers.runCurrent()
onView(allOf(withId(R.id.story_chapter_list))).perform(
scrollToPosition<RecyclerView.ViewHolder>(
2
)
)
onView(
atPositionOnView(
R.id.story_chapter_list,
2,
R.id.chapter_thumbnail
)
).check { view, noViewFoundException ->
var lessonThumbnailImageView = view.findViewById<LessonThumbnailImageView>(
R.id.chapter_thumbnail
)
verify(lessonThumbnailImageView.imageLoader, atLeastOnce()).loadDrawable(
anyInt(),
anyOrNull(),
capture(listCaptor)
)
assertThat(listCaptor.value).contains(ImageTransformation.BLUR)
}
}
}

@Test
fun testStoryFragment_configChange_chapterMissingPrerequisiteThumbnailIsBlurred() {
launch<StoryActivity>(createFractionsStoryActivityIntent()).use {
testCoroutineDispatchers.runCurrent()
onView(isRoot()).perform(orientationLandscape())
onView(allOf(withId(R.id.story_chapter_list))).perform(
scrollToPosition<RecyclerView.ViewHolder>(
2
)
)
onView(
atPositionOnView(
R.id.story_chapter_list,
2,
R.id.chapter_thumbnail
)
).check { view, noViewFoundException ->
var lessonThumbnailImageView = view.findViewById<LessonThumbnailImageView>(
R.id.chapter_thumbnail
)
verify(lessonThumbnailImageView.imageLoader, atLeastOnce()).loadDrawable(
anyInt(),
anyOrNull(),
capture(listCaptor)
)
assertThat(listCaptor.value).contains(ImageTransformation.BLUR)
}
}
}

@Test
fun testStoryFragment_chapterMissingPrerequisiteIsShownCorrectly() {
launch<StoryActivity>(createFractionsStoryActivityIntent()).use {
Expand Down Expand Up @@ -445,7 +524,7 @@ class StoryFragmentTest {
ItemSelectionInputModule::class, MultipleChoiceInputModule::class,
NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, TextInputRuleModule::class,
DragDropSortInputModule::class, ImageClickInputModule::class, InteractionsModule::class,
GcsResourceModule::class, GlideImageLoaderModule::class, ImageParsingModule::class,
GcsResourceModule::class, TestModule::class, ImageParsingModule::class,
HtmlParserEntityTypeModule::class, QuestionModule::class, TestLogReportingModule::class,
TestAccessibilityModule::class, LogStorageModule::class, CachingTestModule::class,
PrimeTopicAssetsControllerModule::class, ExpirationMetaDataRetrieverModule::class,
Expand Down Expand Up @@ -479,4 +558,12 @@ class StoryFragmentTest {

override fun getApplicationInjector(): ApplicationInjector = component
}

/** Provides test dependencies (including a mock for [ImageLoader] to capture its operations). */
@Module
class TestModule {
@Provides
@Singleton
fun provideMockImageLoader() = mock(ImageLoader::class.java)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ import org.oppia.android.testing.RunOn
import org.oppia.android.testing.TestAccessibilityModule
import org.oppia.android.testing.TestCoroutineDispatchers
import org.oppia.android.testing.TestDispatcherModule
import org.oppia.android.testing.TestImageLoaderModule
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.TestPlatform
import org.oppia.android.util.caching.testing.CachingTestModule
import org.oppia.android.util.gcsresource.GcsResourceModule
import org.oppia.android.util.logging.LoggerModule
import org.oppia.android.util.logging.firebase.FirebaseLogUploaderModule
import org.oppia.android.util.parser.GlideImageLoaderModule
import org.oppia.android.util.parser.HtmlParserEntityTypeModule
import org.oppia.android.util.parser.ImageParsingModule
import org.robolectric.annotation.Config
Expand Down Expand Up @@ -350,7 +350,7 @@ class TopicInfoFragmentTest {
ItemSelectionInputModule::class, MultipleChoiceInputModule::class,
NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, TextInputRuleModule::class,
DragDropSortInputModule::class, ImageClickInputModule::class, InteractionsModule::class,
GcsResourceModule::class, GlideImageLoaderModule::class, ImageParsingModule::class,
GcsResourceModule::class, TestImageLoaderModule::class, ImageParsingModule::class,
HtmlParserEntityTypeModule::class, QuestionModule::class, TestLogReportingModule::class,
TestAccessibilityModule::class, LogStorageModule::class, CachingTestModule::class,
PrimeTopicAssetsControllerModule::class, ExpirationMetaDataRetrieverModule::class,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package org.oppia.android.app.utility

import org.mockito.ArgumentCaptor
import org.mockito.Mockito.any

/**
* Returns ArgumentCaptor.capture() as nullable type to avoid java.lang.IllegalStateException
* when null is returned.
*/
fun <T> capture(argumentCaptor: ArgumentCaptor<T>): T = argumentCaptor.capture()

/** Matches anything, including nulls. */
fun <T> anyOrNull(): T = any()
Loading

0 comments on commit 2bcd511

Please sign in to comment.