From 734885a527c72ecf40a3b85c0dd4ab582be8cd3e Mon Sep 17 00:00:00 2001
From: MohitMali <futuretechnoworld.mohit@gmail.com>
Date: Wed, 9 Aug 2023 18:45:35 +0530
Subject: [PATCH] Improved 'Search' Functionality:

* Added a loading progress bar at the end of the RecyclerView when loading more search results as the user scrolls to the bottom. This indicator informs users that additional results are being loaded. The progress bar appears if there are more results available for the search term, providing users with visibility into ongoing loading.
* Enhanced the search loading process for larger ZIM files by introducing coroutines. This background threading approach prevents the UI thread from being impacted and ensures a smooth scrolling experience for users.
---
 .../kiwixmobile/core/search/SearchFragment.kt | 43 ++++++++++++-------
 .../core/search/viewmodel/SearchState.kt      |  9 +++-
 core/src/main/res/layout/fragment_search.xml  | 25 +++++++++--
 .../core/search/viewmodel/SearchStateTest.kt  |  5 ++-
 4 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt
index 5ec1d099b..d10b1bf51 100644
--- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt
+++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt
@@ -38,6 +38,9 @@ import androidx.lifecycle.lifecycleScope
 import androidx.navigation.fragment.findNavController
 import androidx.recyclerview.widget.LinearLayoutManager
 import androidx.recyclerview.widget.RecyclerView
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
 import org.kiwix.kiwixmobile.core.R
 import org.kiwix.kiwixmobile.core.base.BaseActivity
 import org.kiwix.kiwixmobile.core.base.BaseFragment
@@ -146,20 +149,27 @@ class SearchFragment : BaseFragment() {
     if (isDataLoading) return
     val safeStartIndex = searchAdapter?.itemCount ?: 0
     isDataLoading = true
-    val fetchMoreSearchResults = searchState?.getVisibleResults(safeStartIndex)
-    isDataLoading = when {
-      fetchMoreSearchResults == null -> true
-      fetchMoreSearchResults.isEmpty() -> false
-      else -> {
-        val nonDuplicateResults = fetchMoreSearchResults.filter { newItem ->
-          searchAdapter?.items?.none { it == newItem } ?: true
-        }
+    fragmentSearchBinding?.loadingMoreDataIndicator?.isShowing(true)
 
-        if (nonDuplicateResults.isNotEmpty()) {
-          searchAdapter?.addData(nonDuplicateResults)
-          false
-        } else {
-          true
+    CoroutineScope(Dispatchers.IO).launch {
+      val fetchMoreSearchResults = searchState?.getVisibleResults(safeStartIndex)
+      CoroutineScope(Dispatchers.Main).launch {
+        fragmentSearchBinding?.loadingMoreDataIndicator?.isShowing(false)
+        isDataLoading = when {
+          fetchMoreSearchResults == null -> true
+          fetchMoreSearchResults.isEmpty() -> false
+          else -> {
+            val nonDuplicateResults = fetchMoreSearchResults.filter { newItem ->
+              searchAdapter?.items?.none { it == newItem } ?: true
+            }
+
+            if (nonDuplicateResults.isNotEmpty()) {
+              searchAdapter?.addData(nonDuplicateResults)
+              false
+            } else {
+              true
+            }
+          }
         }
       }
     }
@@ -248,15 +258,16 @@ class SearchFragment : BaseFragment() {
     )
   }
 
-  private fun render(state: SearchState) {
+  private suspend fun render(state: SearchState) {
     isDataLoading = false
     searchState = state
     searchInTextMenuItem?.isVisible = state.searchOrigin == FromWebView
     searchInTextMenuItem?.isEnabled = state.searchTerm.isNotBlank()
     fragmentSearchBinding?.searchLoadingIndicator?.isShowing(state.isLoading)
+    val searchResult = state.getVisibleResults(0)
     fragmentSearchBinding?.searchNoResults?.isVisible =
-      state.getVisibleResults(0)?.isEmpty() == true
-    state.getVisibleResults(0)?.let {
+      searchResult?.isEmpty() == true
+    searchResult?.let {
       searchAdapter?.items = it
     }
   }
diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt
index ef745ae6d..f3d62ce88 100644
--- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt
+++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchState.kt
@@ -26,10 +26,13 @@ data class SearchState(
   val recentResults: List<SearchListItem.RecentSearchListItem>,
   val searchOrigin: SearchOrigin
 ) {
-  fun getVisibleResults(startIndex: Int): List<SearchListItem.RecentSearchListItem>? =
+  private var isDataLoading = false
+  suspend fun getVisibleResults(startIndex: Int): List<SearchListItem.RecentSearchListItem>? =
     if (searchTerm.isEmpty()) {
+      isDataLoading = false
       recentResults
     } else {
+      isDataLoading = true
       searchResultsWithTerm.suggestionSearch?.let {
         val maximumResults = it.estimatedMatches
         val safeEndIndex =
@@ -46,13 +49,15 @@ data class SearchState(
          * We check this in SearchFragment to avoid unnecessary data loading
          * while scrolling to the end of the list when there are no items available.
          */
+        isDataLoading = false
         searchResults.ifEmpty { null }
       } ?: kotlin.run {
+        isDataLoading = false
         recentResults
       }
     }
 
-  val isLoading = searchTerm != searchResultsWithTerm.searchTerm
+  val isLoading get() = searchTerm != searchResultsWithTerm.searchTerm || isDataLoading
 }
 
 enum class SearchOrigin {
diff --git a/core/src/main/res/layout/fragment_search.xml b/core/src/main/res/layout/fragment_search.xml
index a7e400d11..7f9592511 100644
--- a/core/src/main/res/layout/fragment_search.xml
+++ b/core/src/main/res/layout/fragment_search.xml
@@ -1,4 +1,5 @@
 <androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:app="http://schemas.android.com/apk/res-auto"
   xmlns:tools="http://schemas.android.com/tools"
   android:layout_width="match_parent"
   android:layout_height="match_parent"
@@ -6,7 +7,7 @@
   android:fitsSystemWindows="true"
   android:orientation="vertical">
 
-  <LinearLayout
+  <androidx.constraintlayout.widget.ConstraintLayout
     android:layout_width="match_parent"
     android:layout_height="match_parent"
     android:orientation="vertical">
@@ -15,11 +16,27 @@
 
     <androidx.recyclerview.widget.RecyclerView
       android:id="@+id/search_list"
-      android:layout_width="match_parent"
-      android:layout_height="match_parent"
+      android:layout_width="0dp"
+      android:layout_height="0dp"
+      app:layout_constraintBottom_toTopOf="@+id/loadingMoreDataIndicator"
+      app:layout_constraintEnd_toEndOf="parent"
+      app:layout_constraintStart_toStartOf="parent"
+      app:layout_constraintTop_toTopOf="parent"
+      android:layout_marginTop="?actionBarSize"
       tools:listitem="@layout/list_item_search" />
 
-  </LinearLayout>
+    <androidx.core.widget.ContentLoadingProgressBar
+      android:id="@+id/loadingMoreDataIndicator"
+      style="?android:attr/progressBarStyleLarge"
+      android:layout_width="40dp"
+      android:layout_height="40dp"
+      android:layout_gravity="center"
+      app:layout_constraintBottom_toBottomOf="parent"
+      app:layout_constraintEnd_toEndOf="parent"
+      app:layout_constraintStart_toStartOf="parent"
+      android:visibility="gone" />
+
+  </androidx.constraintlayout.widget.ConstraintLayout>
 
   <TextView
     android:id="@+id/searchNoResults"
diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt
index b1a0c9528..c71655d09 100644
--- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt
+++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt
@@ -20,6 +20,7 @@ package org.kiwix.kiwixmobile.core.search.viewmodel
 
 import io.mockk.every
 import io.mockk.mockk
+import kotlinx.coroutines.test.runTest
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 import org.kiwix.kiwixmobile.core.search.adapter.SearchListItem.RecentSearchListItem
@@ -28,7 +29,7 @@ import org.kiwix.kiwixmobile.core.search.viewmodel.SearchOrigin.FromWebView
 internal class SearchStateTest {
 
   @Test
-  internal fun `visibleResults use searchResults when searchTerm is not empty`() {
+  internal fun `visibleResults use searchResults when searchTerm is not empty`() = runTest {
     val searchTerm = "notEmpty"
     val suggestionSearchWrapper: SuggestionSearchWrapper = mockk()
     val searchIteratorWrapper: SuggestionIteratorWrapper = mockk()
@@ -58,7 +59,7 @@ internal class SearchStateTest {
   }
 
   @Test
-  internal fun `visibleResults use recentResults when searchTerm is empty`() {
+  internal fun `visibleResults use recentResults when searchTerm is empty`() = runTest {
     val results = listOf(RecentSearchListItem(""))
     assertThat(
       SearchState(