Fix race condition of showing empty view in timetable (#2486)

Co-authored-by: Mikołaj Pich <m.pich@outlook.com>

app/src/main/java/io/github/wulkanowy/ui/modules/luckynumber/history/LuckyNumberHistoryAdapter.kt

@@ -33,4 +33,4 @@ class LuckyNumberHistoryAdapter @Inject constructor() :
     }
 
     class ItemViewHolder(val binding: ItemLuckyNumberHistoryBinding) : RecyclerView.ViewHolder(binding.root)
-}
+}

app/src/main/java/io/github/wulkanowy/ui/modules/timetable/TimetableAdapter.kt

@@ -7,20 +7,20 @@ import android.view.ViewGroup
 import android.widget.TextView
 import androidx.core.view.isVisible
 import androidx.recyclerview.widget.DiffUtil
-import androidx.recyclerview.widget.ListAdapter
 import androidx.recyclerview.widget.RecyclerView
 import io.github.wulkanowy.R
 import io.github.wulkanowy.data.db.entities.Timetable
 import io.github.wulkanowy.databinding.ItemTimetableBinding
 import io.github.wulkanowy.databinding.ItemTimetableEmptyBinding
 import io.github.wulkanowy.databinding.ItemTimetableSmallBinding
+import io.github.wulkanowy.utils.SyncListAdapter
 import io.github.wulkanowy.utils.getPlural
 import io.github.wulkanowy.utils.getThemeAttrColor
 import io.github.wulkanowy.utils.toFormattedString
 import javax.inject.Inject
 
 class TimetableAdapter @Inject constructor() :
-    ListAdapter<TimetableItem, RecyclerView.ViewHolder>(differ) {
+    SyncListAdapter<TimetableItem, RecyclerView.ViewHolder>(Differ) {
 
     override fun getItemViewType(position: Int): Int = getItem(position).type.ordinal
 
@@ -61,12 +61,10 @@ class TimetableAdapter @Inject constructor() :
                 binding = holder.binding,
                 item = getItem(position) as TimetableItem.Small,
             )
-
             is NormalViewHolder -> bindNormalView(
                 binding = holder.binding,
                 item = getItem(position) as TimetableItem.Normal,
             )
-
             is EmptyViewHolder -> bindEmptyView(
                 binding = holder.binding,
                 item = getItem(position) as TimetableItem.Empty,
@@ -307,31 +305,29 @@ class TimetableAdapter @Inject constructor() :
     private class EmptyViewHolder(val binding: ItemTimetableEmptyBinding) :
         RecyclerView.ViewHolder(binding.root)
 
-    companion object {
+    private object Differ : DiffUtil.ItemCallback<TimetableItem>() {
-        private val differ = object : DiffUtil.ItemCallback<TimetableItem>() {
+        override fun areItemsTheSame(oldItem: TimetableItem, newItem: TimetableItem): Boolean =
-            override fun areItemsTheSame(oldItem: TimetableItem, newItem: TimetableItem): Boolean =
+            when {
-                when {
+                oldItem is TimetableItem.Small && newItem is TimetableItem.Small -> {
-                    oldItem is TimetableItem.Small && newItem is TimetableItem.Small -> {
+                    oldItem.lesson.start == newItem.lesson.start
-                        oldItem.lesson.start == newItem.lesson.start
-                    }
-
-                    oldItem is TimetableItem.Normal && newItem is TimetableItem.Normal -> {
-                        oldItem.lesson.start == newItem.lesson.start
-                    }
-
-                    else -> oldItem == newItem
                 }
 
-            override fun areContentsTheSame(oldItem: TimetableItem, newItem: TimetableItem) =
+                oldItem is TimetableItem.Normal && newItem is TimetableItem.Normal -> {
-                oldItem == newItem
+                    oldItem.lesson.start == newItem.lesson.start
+                }
 
-            override fun getChangePayload(oldItem: TimetableItem, newItem: TimetableItem): Any? {
+                else -> oldItem == newItem
-                return if (oldItem is TimetableItem.Normal && newItem is TimetableItem.Normal) {
-                    if (oldItem.lesson == newItem.lesson && oldItem.showGroupsInPlan == newItem.showGroupsInPlan && oldItem.timeLeft != newItem.timeLeft) {
-                        "time_left"
-                    } else super.getChangePayload(oldItem, newItem)
-                } else super.getChangePayload(oldItem, newItem)
             }
+
+        override fun areContentsTheSame(oldItem: TimetableItem, newItem: TimetableItem) =
+            oldItem == newItem
+
+        override fun getChangePayload(oldItem: TimetableItem, newItem: TimetableItem): Any? {
+            return if (oldItem is TimetableItem.Normal && newItem is TimetableItem.Normal) {
+                if (oldItem.lesson == newItem.lesson && oldItem.showGroupsInPlan == newItem.showGroupsInPlan && oldItem.timeLeft != newItem.timeLeft) {
+                    "time_left"
+                } else super.getChangePayload(oldItem, newItem)
+            } else super.getChangePayload(oldItem, newItem)
         }
     }
 }

app/src/main/java/io/github/wulkanowy/ui/modules/timetable/TimetableFragment.kt

@@ -21,7 +21,11 @@ import io.github.wulkanowy.ui.modules.main.MainView
 import io.github.wulkanowy.ui.modules.timetable.additional.AdditionalLessonsFragment
 import io.github.wulkanowy.ui.modules.timetable.completed.CompletedLessonsFragment
 import io.github.wulkanowy.ui.widgets.DividerItemDecoration
-import io.github.wulkanowy.utils.*
+import io.github.wulkanowy.utils.dpToPx
+import io.github.wulkanowy.utils.firstSchoolDayInSchoolYear
+import io.github.wulkanowy.utils.getThemeAttrColor
+import io.github.wulkanowy.utils.lastSchoolDayInSchoolYear
+import io.github.wulkanowy.utils.openMaterialDatePicker
 import java.time.LocalDate
 import javax.inject.Inject
 
@@ -104,8 +108,11 @@ class TimetableFragment : BaseFragment<FragmentTimetableBinding>(R.layout.fragme
         }
     }
 
-    override fun updateData(data: List<TimetableItem>) {
+    override fun updateData(data: List<TimetableItem>, isDayChanged: Boolean) {
-        timetableAdapter.submitList(data)
+        when {
+            isDayChanged -> timetableAdapter.recreate(data)
+            else -> timetableAdapter.submitList(data)
+        }
     }
 
     override fun clearData() {

app/src/main/java/io/github/wulkanowy/ui/modules/timetable/TimetablePresenter.kt

@@ -81,7 +81,7 @@ class TimetablePresenter @Inject constructor(
         } else currentDate?.previousSchoolDay
 
         reloadView(date ?: return)
-        loadData()
+        loadData(isDayChanged = true)
     }
 
     fun onNextDay() {
@@ -90,7 +90,7 @@ class TimetablePresenter @Inject constructor(
         } else currentDate?.nextSchoolDay
 
         reloadView(date ?: return)
-        loadData()
+        loadData(isDayChanged = true)
     }
 
     fun onPickDate() {
@@ -104,7 +104,7 @@ class TimetablePresenter @Inject constructor(
 
     fun onSwipeRefresh() {
         Timber.i("Force refreshing the timetable")
-        loadData(true)
+        loadData(forceRefresh = true)
     }
 
     fun onRetry() {
@@ -112,7 +112,7 @@ class TimetablePresenter @Inject constructor(
             showErrorView(false)
             showProgress(true)
         }
-        loadData(true)
+        loadData(forceRefresh = true)
     }
 
     fun onDetailsClick() {
@@ -145,7 +145,7 @@ class TimetablePresenter @Inject constructor(
         return true
     }
 
-    private fun loadData(forceRefresh: Boolean = false) {
+    private fun loadData(forceRefresh: Boolean = false, isDayChanged: Boolean = false) {
         flatResourceFlow {
             val student = studentRepository.getCurrentStudent()
             val semester = semesterRepository.getCurrentSemester(student)
@@ -169,9 +169,9 @@ class TimetablePresenter @Inject constructor(
                     enableSwipe(true)
                     showProgress(false)
                     showErrorView(false)
+                    updateData(it.lessons, isDayChanged)
                     showContent(it.lessons.isNotEmpty())
                     showEmpty(it.lessons.isEmpty())
-                    updateData(it.lessons)
                     setDayHeaderMessage(it.headers.find { header -> header.date == currentDate }?.content)
                     reloadNavigation()
                 }
@@ -216,15 +216,14 @@ class TimetablePresenter @Inject constructor(
         }
     }
 
-    private fun updateData(lessons: List<Timetable>) {
+    private fun updateData(lessons: List<Timetable>, isDayChanged: Boolean) {
         tickTimer?.cancel()
 
-        if (currentDate != now()) {
+        view?.updateData(createItems(lessons), isDayChanged)
-            view?.updateData(createItems(lessons))
+        if (currentDate == now()) {
-        } else {
+            tickTimer = timer(period = 2_000, initialDelay = 2_000) {
-            tickTimer = timer(period = 2_000) {
                 Handler(Looper.getMainLooper()).post {
-                    view?.updateData(createItems(lessons))
+                    view?.updateData(createItems(lessons), isDayChanged)
                 }
             }
         }

app/src/main/java/io/github/wulkanowy/ui/modules/timetable/TimetableView.kt

@@ -12,7 +12,7 @@ interface TimetableView : BaseView {
 
     fun initView()
 
-    fun updateData(data: List<TimetableItem>)
+    fun updateData(data: List<TimetableItem>, isDayChanged: Boolean)
 
     fun updateNavigationDay(date: String)
 

app/src/main/java/io/github/wulkanowy/utils/SyncListAdapter.kt

@@ -0,0 +1,66 @@
+package io.github.wulkanowy.utils
+
+import android.annotation.SuppressLint
+import androidx.recyclerview.widget.DiffUtil
+import androidx.recyclerview.widget.RecyclerView
+
+/**
+ * Custom alternative to androidx.recyclerview.widget.ListAdapter. ListAdapter is asynchronous which
+ * caused data race problems in views when a Resource.Error arrived shortly after
+ * Resource.Intermediate/Success - occasionally in that case the user could see both the Resource's
+ * data and an error message one on top of the other. This is synchronized by design to avoid that
+ * problem, however it retains the quality of life improvements of the original.
+ */
+abstract class SyncListAdapter<T : Any, VH : RecyclerView.ViewHolder> private constructor(
+    private val updateStrategy: SyncListAdapter<T, VH>.(List<T>) -> Unit
+) : RecyclerView.Adapter<VH>() {
+
+    constructor(differ: DiffUtil.ItemCallback<T>) : this({ newItems ->
+        val diffResult = DiffUtil.calculateDiff(toCallback(differ, items, newItems))
+        items = newItems
+        diffResult.dispatchUpdatesTo(this)
+    })
+
+    var items = emptyList<T>()
+        private set
+
+    final override fun getItemCount() = items.size
+
+    fun getItem(position: Int): T {
+        return items[position]
+    }
+
+    /**
+     * Updates all items, same as submitList, however also disables animations temporarily.
+     * This prevents a flashing effect on some views. Should be used in favor of submitList when
+     * all data is changed (e.g. the selected day changes in timetable causing all lessons to change).
+     */
+    @SuppressLint("NotifyDataSetChanged")
+    fun recreate(data: List<T>) {
+        items = data
+        notifyDataSetChanged()
+    }
+
+    fun submitList(data: List<T>) {
+        updateStrategy(data.toList())
+    }
+
+    private fun <T : Any> toCallback(
+        itemCallback: DiffUtil.ItemCallback<T>,
+        old: List<T>,
+        new: List<T>,
+    ) = object : DiffUtil.Callback() {
+        override fun getOldListSize() = old.size
+
+        override fun getNewListSize() = new.size
+
+        override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int) =
+            itemCallback.areItemsTheSame(old[oldItemPosition], new[newItemPosition])
+
+        override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int) =
+            itemCallback.areContentsTheSame(old[oldItemPosition], new[newItemPosition])
+
+        override fun getChangePayload(oldItemPosition: Int, newItemPosition: Int) =
+            itemCallback.getChangePayload(old[oldItemPosition], new[newItemPosition])
+    }
+}