Fix #4449: Modify the design of the chapter list in lessons tab (#4535)

* add locked lesson view, modify completed lesson view * add borders, not started chapter view, recycler multi adapter * add divider item decoration * commit before checkout -- all design modified * add divider item decoration grey color * modify chapter recycler items to have 0 margins * remove unused lines * add in-progress chapter view item * use frame layout to make dotted line thinner * lint * remove unused resources and code analysis * lint * rename file to more describing name * add tests and correct some errors * suggested changes + fix some problems * rename resources * lint * lint * add missing eof * add private identifier to enum * remove 0dp margin * commit requested changes * add framelayout to sw600dp-land for tablets + lint fix for file * rename resources * lint * switch to constraint layout * remove the "period sign" in chapter index. * add field to topicsummary proto, and complete the func for opening chapter list by default * lint * update test * update logic * merge with develop and fix minor bug * these two tests together validate that chapter list is expanded by default * lint * ci fail check * nits * add suggested changes
oppia · Sep 20, 2022 · e91de5d · e91de5d
1 parent 91014af
commit e91de5d
Show file tree

Hide file tree

Showing 20 changed files with 597 additions and 168 deletions.
diff --git a/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/home/HomeFragmentPresenter.kt
@@ -45,7 +45,7 @@ class HomeFragmentPresenter @Inject constructor(
   private val dateTimeUtil: DateTimeUtil,
   private val translationController: TranslationController
 ) {
-  private val routeToTopicListener = activity as RouteToTopicListener
+  private val routeToTopicPlayStoryListener = activity as RouteToTopicPlayStoryListener
   private lateinit var binding: HomeFragmentBinding
   private var internalProfileId: Int = -1
 
@@ -149,7 +149,11 @@ class HomeFragmentPresenter @Inject constructor(
   }
 
   fun onTopicSummaryClicked(topicSummary: TopicSummary) {
-    routeToTopicListener.routeToTopic(internalProfileId, topicSummary.topicId)
+    routeToTopicPlayStoryListener.routeToTopicPlayStory(
+      internalProfileId,
+      topicSummary.topicId,
+      topicSummary.firstStoryId
+    )
   }
 
   private fun logHomeActivityEvent() {

diff --git a/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt
@@ -17,7 +17,10 @@ import org.oppia.android.app.model.StorySummary
 import org.oppia.android.app.recyclerview.BindableAdapter
 import org.oppia.android.app.topic.RouteToResumeLessonListener
 import org.oppia.android.app.topic.RouteToStoryListener
-import org.oppia.android.databinding.LessonsChapterViewBinding
+import org.oppia.android.databinding.LessonsCompletedChapterViewBinding
+import org.oppia.android.databinding.LessonsInProgressChapterViewBinding
+import org.oppia.android.databinding.LessonsLockedChapterViewBinding
+import org.oppia.android.databinding.LessonsNotStartedChapterViewBinding
 import org.oppia.android.databinding.TopicLessonsFragmentBinding
 import org.oppia.android.databinding.TopicLessonsStorySummaryBinding
 import org.oppia.android.databinding.TopicLessonsTitleBinding
@@ -224,12 +227,50 @@ class TopicLessonsFragmentPresenter @Inject constructor(
   }
 
   private fun createChapterRecyclerViewAdapter(): BindableAdapter<ChapterSummaryViewModel> {
-    return BindableAdapter.SingleTypeBuilder
-      .newBuilder<ChapterSummaryViewModel>()
-      .registerViewDataBinderWithSameModelType(
-        inflateDataBinding = LessonsChapterViewBinding::inflate,
-        setViewModel = LessonsChapterViewBinding::setViewModel
-      ).build()
+    return BindableAdapter.MultiTypeBuilder
+      .newBuilder<ChapterSummaryViewModel, ChapterViewType> { viewModel ->
+        when (viewModel.chapterPlayState) {
+          ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES -> ChapterViewType.CHAPTER_LOCKED
+          ChapterPlayState.COMPLETED -> ChapterViewType.CHAPTER_COMPLETED
+          ChapterPlayState.IN_PROGRESS_SAVED, ChapterPlayState.IN_PROGRESS_NOT_SAVED,
+          ChapterPlayState.STARTED_NOT_COMPLETED, ChapterPlayState.COMPLETION_STATUS_UNSPECIFIED
+          -> ChapterViewType.CHAPTER_IN_PROGRESS
+          ChapterPlayState.NOT_STARTED -> ChapterViewType.CHAPTER_NOT_STARTED
+          ChapterPlayState.UNRECOGNIZED -> throw IllegalArgumentException("Play state unknown")
+        }
+      }
+      .registerViewDataBinder(
+        viewType = ChapterViewType.CHAPTER_LOCKED,
+        inflateDataBinding = LessonsLockedChapterViewBinding::inflate,
+        setViewModel = LessonsLockedChapterViewBinding::setViewModel,
+        transformViewModel = { it }
+      )
+      .registerViewDataBinder(
+        viewType = ChapterViewType.CHAPTER_COMPLETED,
+        inflateDataBinding = LessonsCompletedChapterViewBinding::inflate,
+        setViewModel = LessonsCompletedChapterViewBinding::setViewModel,
+        transformViewModel = { it }
+      )
+      .registerViewDataBinder(
+        viewType = ChapterViewType.CHAPTER_NOT_STARTED,
+        inflateDataBinding = LessonsNotStartedChapterViewBinding::inflate,
+        setViewModel = LessonsNotStartedChapterViewBinding::setViewModel,
+        transformViewModel = { it }
+      )
+      .registerViewDataBinder(
+        viewType = ChapterViewType.CHAPTER_IN_PROGRESS,
+        inflateDataBinding = LessonsInProgressChapterViewBinding::inflate,
+        setViewModel = LessonsInProgressChapterViewBinding::setViewModel,
+        transformViewModel = { it }
+      )
+      .build()
+  }
+
+  private enum class ChapterViewType {
+    CHAPTER_NOT_STARTED,
+    CHAPTER_COMPLETED,
+    CHAPTER_LOCKED,
+    CHAPTER_IN_PROGRESS
   }
 
   fun storySummaryClicked(storySummary: StorySummary) {

diff --git a/app/src/main/res/drawable/chapter_dark_green_bg_with_bright_green_border.xml b/app/src/main/res/drawable/chapter_dark_green_bg_with_bright_green_border.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<shape xmlns:android="http://schemas.android.com/apk/res/android"
+  android:shape="rectangle">
+  <stroke
+    android:width="4dp"
+    android:color="@color/color_def_bright_green" />
+  <solid android:color="@color/color_def_oppia_green" />
+</shape>
diff --git a/app/src/main/res/drawable/chapter_white_bg_with_bright_green_border.xml b/app/src/main/res/drawable/chapter_white_bg_with_bright_green_border.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<shape xmlns:android="http://schemas.android.com/apk/res/android"
+  android:shape="rectangle">
+  <stroke android:color="@color/color_def_bright_green" android:width="4dp"/>
+  <solid android:color="@color/color_def_white"/>
+</shape>
diff --git a/app/src/main/res/drawable/grey_recycler_view_item_decoration_divider.xml b/app/src/main/res/drawable/grey_recycler_view_item_decoration_divider.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<shape xmlns:android="http://schemas.android.com/apk/res/android"
+  android:shape="rectangle">
+  <solid android:color="@color/color_def_accessible_light_grey_2" />
+  <size android:height="1dp" />
+</shape>
diff --git a/app/src/main/res/drawable/ic_baseline_lock_24.xml b/app/src/main/res/drawable/ic_baseline_lock_24.xml
@@ -1,10 +1,5 @@
-<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 android:autoMirrored="true" android:height="24dp"
+    android:tint="#FFFFFF" android:viewportHeight="24"
+    android:viewportWidth="24" android:width="24dp" xmlns:android="http://schemas.android.com/apk/res/android">
+    <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>
diff --git a/app/src/main/res/drawable/ic_pending_24dp.xml b/app/src/main/res/drawable/ic_pending_24dp.xml
@@ -4,15 +4,15 @@
   android:viewportWidth="24"
   android:viewportHeight="24">
   <path
-    android:fillColor="#FF333333"
+    android:fillColor="#FFFFFF"
     android:pathData="M12,2C6.48,2 2,6.48 2,12c0,5.52 4.48,10 10,10s10,-4.48 10,-10C22,6.48 17.52,2 12,2zM12,20c-4.42,0 -8,-3.58 -8,-8c0,-4.42 3.58,-8 8,-8s8,3.58 8,8C20,16.42 16.42,20 12,20z" />
   <path
-    android:fillColor="#FF333333"
+    android:fillColor="#FFFFFF"
     android:pathData="M7,12m-1.5,0a1.5,1.5 0,1 1,3 0a1.5,1.5 0,1 1,-3 0" />
   <path
-    android:fillColor="#FF333333"
+    android:fillColor="#FFFFFF"
     android:pathData="M12,12m-1.5,0a1.5,1.5 0,1 1,3 0a1.5,1.5 0,1 1,-3 0" />
   <path
-    android:fillColor="#FF333333"
+    android:fillColor="#FFFFFF"
     android:pathData="M17,12m-1.5,0a1.5,1.5 0,1 1,3 0a1.5,1.5 0,1 1,-3 0" />
 </vector>
diff --git a/app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml b/app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml
@@ -49,8 +49,8 @@
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
             android:layout_marginTop="24dp"
-            android:gravity="center"
             android:contentDescription="@{viewModel.computeProgressContainerContentDescription()}"
+            android:gravity="center"
             android:orientation="vertical">
 
             <FrameLayout
@@ -141,37 +141,37 @@
                 android:layout_height="wrap_content"
                 android:layout_gravity="center_horizontal|bottom"
                 android:contentDescription="@{isListExpanded ? @string/hide_chapter_list : @string/show_chapter_list}"
-                app:srcCompat="@drawable/ic_arrow_drop_down_black_24dp"
                 app:isRotationAnimationClockwise="@{isListExpanded}"
-                app:rotationAnimationAngle="@{180f}" />
+                app:rotationAnimationAngle="@{180f}"
+                app:srcCompat="@drawable/ic_arrow_drop_down_black_24dp" />
             </FrameLayout>
           </LinearLayout>
         </LinearLayout>
-
-        <LinearLayout
+        <!-- FrameLayout is used here instead of LinearLayout to properly adjust the spacing of the
+            dashed divider by allowing the divider to render slightly on top of the container. -->
+        <FrameLayout
           android:id="@+id/chapter_list_container"
           android:layout_width="match_parent"
           android:layout_height="wrap_content"
           android:background="@color/color_def_white_light"
           android:orientation="vertical"
           android:visibility="@{isListExpanded? View.VISIBLE : View.GONE}">
 
-          <View
-            android:id="@+id/topic_play_story_dashed_line_view"
-            android:layout_width="match_parent"
-            android:layout_height="2dp"
-            android:layout_gravity="center_horizontal"
-            android:background="@drawable/dashed_divider" />
-
           <androidx.recyclerview.widget.RecyclerView
             android:id="@+id/chapter_recycler_view"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
-            android:layout_marginTop="8dp"
-            android:layout_marginBottom="8dp"
+            android:layout_marginTop="2dp"
             app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
             app:list="@{viewModel.chapterSummaryItemList}" />
-        </LinearLayout>
+
+          <View
+            android:id="@+id/topic_play_story_dashed_line_view"
+            android:layout_width="match_parent"
+            android:layout_height="2dp"
+            android:layout_gravity="center_horizontal"
+            android:background="@drawable/dashed_divider" />
+        </FrameLayout>
       </LinearLayout>
     </com.google.android.material.card.MaterialCardView>
   </FrameLayout>

diff --git a/app/src/main/res/layout/lessons_chapter_view.xml b/app/src/main/res/layout/lessons_chapter_view.xml
diff --git a/app/src/main/res/layout/lessons_completed_chapter_view.xml b/app/src/main/res/layout/lessons_completed_chapter_view.xml
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="utf-8"?>
+<layout xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:app="http://schemas.android.com/apk/res-auto">
+
+  <data>
+
+    <import type="android.view.View" />
+
+    <import type="org.oppia.android.app.model.ChapterPlayState" />
+
+    <variable
+      name="viewModel"
+      type="org.oppia.android.app.topic.lessons.ChapterSummaryViewModel" />
+  </data>
+
+  <androidx.constraintlayout.widget.ConstraintLayout
+    android:id="@+id/lessons_completed_chapter_view"
+    android:layout_width="match_parent"
+    android:layout_height="wrap_content"
+    android:onClick="@{() -> viewModel.onClick(viewModel.explorationId)}"
+    android:orientation="vertical">
+
+    <TextView
+      android:id="@+id/chapter_index"
+      android:layout_width="wrap_content"
+      android:layout_height="match_parent"
+      android:background="@color/color_def_oppia_green"
+      android:fontFamily="sans-serif"
+      android:gravity="center"
+      android:importantForAccessibility="@{viewModel.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.IMPORTANT_FOR_ACCESSIBILITY_YES : View.IMPORTANT_FOR_ACCESSIBILITY_NO}"
+      android:minWidth="60dp"
+      android:minHeight="48dp"
+      android:paddingStart="8dp"
+      android:paddingTop="12dp"
+      android:paddingEnd="8dp"
+      android:paddingBottom="12dp"
+      android:text="@{viewModel.computePlayChapterIndexText()}"
+      android:textColor="@color/color_def_white"
+      android:textSize="20sp"
+      app:layout_constraintStart_toStartOf="parent"
+      app:layout_constraintTop_toTopOf="parent" />
+
+    <TextView
+      android:id="@+id/chapter_name"
+      style="@style/TextViewStart"
+      android:layout_width="0dp"
+      android:layout_height="0dp"
+      android:background="@color/color_def_light_green"
+      android:fontFamily="sans-serif"
+      android:gravity="center|start"
+      android:importantForAccessibility="@{viewModel.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.IMPORTANT_FOR_ACCESSIBILITY_YES : View.IMPORTANT_FOR_ACCESSIBILITY_NO}"
+      android:minHeight="48dp"
+      android:paddingStart="12dp"
+      android:paddingEnd="12dp"
+      android:text="@{viewModel.chapterTitle}"
+      android:textColor="@{viewModel.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? @color/oppia_primary_text : @color/oppia_primary_text_30}"
+      android:textSize="14sp"
+      app:layout_constraintBottom_toTopOf="@id/divider"
+      app:layout_constraintEnd_toEndOf="parent"
+      app:layout_constraintStart_toEndOf="@id/chapter_index"
+      app:layout_constraintTop_toTopOf="parent" />
+
+    <View
+      android:id="@+id/divider"
+      android:layout_width="match_parent"
+      android:layout_height="1dp"
+      android:background="@drawable/grey_recycler_view_item_decoration_divider"
+      app:layout_constraintBottom_toBottomOf="parent" />
+  </androidx.constraintlayout.widget.ConstraintLayout>
+</layout>