Clean up some unnecessary coroutine usage and missing back button handling

app/src/main/java/com/wbrawner/trainterval/activetimer/ActiveTimerFragment.kt

@@ -2,8 +2,10 @@ package com.wbrawner.trainterval.activetimer
 
 import android.os.Bundle
 import android.view.LayoutInflater
+import android.view.MenuItem
 import android.view.View
 import android.view.ViewGroup
+import androidx.appcompat.app.AppCompatActivity
 import androidx.fragment.app.Fragment
 import androidx.lifecycle.Observer
 import androidx.navigation.fragment.findNavController
@@ -24,8 +26,16 @@ class ActiveTimerFragment : Fragment() {
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         timerId = requireArguments().getLong("timerId")
+        setHasOptionsMenu(true)
     }
 
+    override fun onOptionsItemSelected(item: MenuItem): Boolean =
+        if (item.itemId == android.R.id.home) {
+            findNavController().navigateUp()
+        } else {
+            super.onOptionsItemSelected(item)
+        }
+
     override fun onCreateView(
         inflater: LayoutInflater, container: ViewGroup?,
         savedInstanceState: Bundle?
@@ -33,6 +43,10 @@ class ActiveTimerFragment : Fragment() {
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
+        (activity as? AppCompatActivity)?.let {
+            it.setSupportActionBar(toolbar)
+            it.supportActionBar?.setDisplayHomeAsUpEnabled(true)
+        }
         coroutineScope = CoroutineScope(Dispatchers.Main)
         coroutineScope!!.launch {
             activeTimerViewModel.timerState.observe(viewLifecycleOwner, Observer { state ->
@@ -42,15 +56,23 @@ class ActiveTimerFragment : Fragment() {
                     is IntervalTimerActiveState.ExitState -> findNavController().navigateUp()
                 }
             })
-        }
-        coroutineScope!!.launch {
             activeTimerViewModel.init(timerId)
         }
+        skipPreviousButton.setOnClickListener {
+            coroutineScope!!.launch {
+                activeTimerViewModel.goBack()
+            }
+        }
         playPauseButton.setOnClickListener {
             coroutineScope!!.launch {
                 activeTimerViewModel.toggleTimer()
             }
         }
+        skipNextButton.setOnClickListener {
+            coroutineScope!!.launch {
+                activeTimerViewModel.skipAhead()
+            }
+        }
     }
 
     private fun renderLoading() {

app/src/main/java/com/wbrawner/trainterval/activetimer/ActiveTimerViewModel.kt

@@ -12,7 +12,6 @@ import com.wbrawner.trainterval.model.IntervalTimerDao
 import com.wbrawner.trainterval.model.Phase
 import com.wbrawner.trainterval.toIntervalDuration
 import kotlinx.coroutines.*
-import kotlin.coroutines.coroutineContext
 
 class ActiveTimerViewModel(
     private val logger: Logger,
@@ -59,42 +58,42 @@ class ActiveTimerViewModel(
                 )
             )
         } else {
-            coroutineScope {
-                timerJob = launch {
-                    startTimer()
-                }
-            }
+            startTimer()
         }
     }
 
     private suspend fun startTimer() {
-        timerRunning = true
-        timerState.postValue(
-            TimerRunningState(
-                timerRunning,
-                timeRemaining.toIntervalDuration().toString(),
-                currentSet,
-                timer.sets,
-                currentRound,
-                timer.cycles
-            )
-        )
-        while (coroutineContext.isActive && timerRunning) {
-            delay(1_000)
-            timeRemaining -= 1_000
-            if (timeRemaining <= 0) {
-                goForward()
-            }
-            timerState.postValue(
-                TimerRunningState(
-                    timerRunning,
-                    timeRemaining.toIntervalDuration().toString(),
-                    currentSet,
-                    timer.sets,
-                    currentRound,
-                    timer.cycles
+        coroutineScope {
+            timerJob = launch {
+                timerRunning = true
+                timerState.postValue(
+                    TimerRunningState(
+                        timerRunning,
+                        timeRemaining.toIntervalDuration().toString(),
+                        currentSet,
+                        timer.sets,
+                        currentRound,
+                        timer.cycles
+                    )
                 )
-            )
+                while (coroutineContext.isActive && timerRunning) {
+                    delay(1_000)
+                    timeRemaining -= 1_000
+                    if (timeRemaining <= 0) {
+                        goForward()
+                    }
+                    timerState.postValue(
+                        TimerRunningState(
+                            timerRunning,
+                            timeRemaining.toIntervalDuration().toString(),
+                            currentSet,
+                            timer.sets,
+                            currentRound,
+                            timer.cycles
+                        )
+                    )
+                }
+            }
         }
     }
 

app/src/main/java/com/wbrawner/trainterval/timerform/TimerFormFragment.kt

@@ -2,6 +2,7 @@ package com.wbrawner.trainterval.timerform
 
 import android.os.Bundle
 import android.view.LayoutInflater
+import android.view.MenuItem
 import android.view.View
 import android.view.ViewGroup
 import androidx.appcompat.app.AppCompatActivity
@@ -23,6 +24,18 @@ class TimerFormFragment : Fragment() {
     private var coroutineScope: CoroutineScope? = null
     private val timerFormViewModel: TimerFormViewModel by inject()
 
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setHasOptionsMenu(true)
+    }
+
+    override fun onOptionsItemSelected(item: MenuItem): Boolean =
+        if (item.itemId == android.R.id.home) {
+            findNavController().navigateUp()
+        } else {
+            super.onOptionsItemSelected(item)
+        }
+
     override fun onCreateView(
         inflater: LayoutInflater, container: ViewGroup?,
         savedInstanceState: Bundle?

app/src/main/java/com/wbrawner/trainterval/timerlist/TimerListAdapter.kt

@@ -8,12 +8,9 @@ import androidx.recyclerview.widget.DiffUtil
 import androidx.recyclerview.widget.ListAdapter
 import androidx.recyclerview.widget.RecyclerView
 import com.wbrawner.trainterval.model.IntervalTimer
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
 
 class TimerListAdapter(
-    private val timerListViewModel: TimerListViewModel,
-    private val coroutineScope: CoroutineScope
+    private val timerListViewModel: TimerListViewModel
 ) : ListAdapter<IntervalTimer, TimerListAdapter.ViewHolder>(IntervalTimerDiffUtilCallback()) {
 
     override fun onCreateViewHolder(parent: ViewGroup, viewType: Int) = ViewHolder(
@@ -26,9 +23,7 @@ class TimerListAdapter(
         holder.title.text = timer.name
         holder.description.text = timer.description
         holder.itemView.setOnClickListener {
-            coroutineScope.launch {
-                timerListViewModel.openTimer(timer)
-            }
+            timerListViewModel.openTimer(timer)
         }
     }
 

app/src/main/java/com/wbrawner/trainterval/timerlist/TimerListFragment.kt

@@ -1,6 +1,7 @@
 package com.wbrawner.trainterval.timerlist
 
 import android.os.Bundle
+import android.util.Log
 import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
@@ -14,15 +15,10 @@ import com.wbrawner.trainterval.R
 import com.wbrawner.trainterval.model.IntervalTimer
 import com.wbrawner.trainterval.timerlist.IntervalTimerListState.*
 import kotlinx.android.synthetic.main.fragment_timer_list.*
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.cancel
-import kotlinx.coroutines.launch
 import org.koin.android.ext.android.inject
 
 class TimerListFragment : Fragment() {
 
-    private var coroutineScope: CoroutineScope? = null
     private val timerListViewModel: TimerListViewModel by inject()
 
     override fun onCreate(savedInstanceState: Bundle?) {
@@ -37,7 +33,6 @@ class TimerListFragment : Fragment() {
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
-        coroutineScope = CoroutineScope(Dispatchers.Main)
         (activity as? AppCompatActivity)?.let {
             it.setSupportActionBar(toolbar)
             it.supportActionBar?.setDisplayHomeAsUpEnabled(false)
@@ -49,31 +44,26 @@ class TimerListFragment : Fragment() {
                 LinearLayoutManager.VERTICAL
             )
         )
-        timerList.adapter = TimerListAdapter(timerListViewModel, coroutineScope!!)
+        timerList.adapter = TimerListAdapter(timerListViewModel)
         addTimerButton.setOnClickListener {
-            coroutineScope?.launch {
-                timerListViewModel.addTimer()
+            timerListViewModel.addTimer()
+        }
+        timerListViewModel.timerState.observe(viewLifecycleOwner, Observer { state ->
+            Log.d("TimerListFragment", "Received state $state")
+            when (state) {
+                is LoadingState -> renderLoading()
+                is EmptyListState -> renderEmptyList()
+                is SuccessListState -> renderSuccessState(state.timers)
+                is ErrorState -> renderErrorState(state.message)
+                is CreateTimer -> findNavController().navigate(R.id.timerFormFragment)
+                is EditTimer -> findNavController().navigate(R.id.timerFormFragment)
+                is OpenTimer -> findNavController().navigate(
+                    R.id.activeTimerFragment,
+                    Bundle().apply { putLong("timerId", state.timerId) }
+                )
             }
-        }
-        coroutineScope!!.launch {
-            timerListViewModel.timerState.observe(viewLifecycleOwner, Observer { state ->
-                when (state) {
-                    is LoadingState -> renderLoading()
-                    is EmptyListState -> renderEmptyList()
-                    is SuccessListState -> renderSuccessState(state.timers)
-                    is ErrorState -> renderErrorState(state.message)
-                    is CreateTimer -> findNavController().navigate(R.id.timerFormFragment)
-                    is EditTimer -> findNavController().navigate(R.id.timerFormFragment)
-                    is OpenTimer -> findNavController().navigate(
-                        R.id.activeTimerFragment,
-                        Bundle().apply { putLong("timerId", state.timerId) }
-                    )
-                }
-            })
-        }
-        coroutineScope!!.launch {
-            timerListViewModel.init()
-        }
+        })
+        timerListViewModel.init()
     }
 
     private fun renderLoading() {
@@ -106,10 +96,4 @@ class TimerListFragment : Fragment() {
         progressBar.visibility = View.GONE
         timerList.visibility = View.GONE
     }
-
-    override fun onDestroyView() {
-        coroutineScope?.cancel()
-        coroutineScope = null
-        super.onDestroyView()
-    }
 }

app/src/main/java/com/wbrawner/trainterval/timerlist/TimerListViewModel.kt

@@ -37,8 +37,13 @@ class TimerListViewModel(
         }
     }
 
-    suspend fun addTimer() {
-        timerState.postValue(CreateTimer)
+    fun addTimer() {
+        timerState.value = CreateTimer
+        if (timers.isEmpty()) {
+            timerState.postValue(EmptyListState)
+        } else {
+            timerState.postValue(SuccessListState(timers))
+        }
     }
 
     suspend fun editTimer(timer: IntervalTimer) {
@@ -53,8 +58,13 @@ class TimerListViewModel(
 
     }
 
-    suspend fun openTimer(timer: IntervalTimer) {
-
+    fun openTimer(timer: IntervalTimer) {
+        timerState.value = OpenTimer(timer.id!!)
+        if (timers.isEmpty()) {
+            timerState.postValue(EmptyListState)
+        } else {
+            timerState.postValue(SuccessListState(timers))
+        }
     }
 }
 

app/src/main/res/drawable/ic_pause.xml

@@ -4,6 +4,6 @@
         android:viewportWidth="24.0"
         android:viewportHeight="24.0">
     <path
-        android:fillColor="#FF000000"
+        android:fillColor="@color/colorOnSurface"
         android:pathData="M6,19h4L10,5L6,5v14zM14,5v14h4L18,5h-4z"/>
 </vector>

app/src/main/res/layout/fragment_active_timer.xml

@@ -11,23 +11,20 @@
         android:id="@+id/appBarLayout"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-        android:background="@android:color/transparent"
-        app:layout_constraintTop_toTopOf="parent">
+        android:background="#00000000"
+        android:clipChildren="false"
+        android:clipToPadding="false"
+        android:elevation="0dp"
+        app:elevation="0dp">
 
-        <com.google.android.material.card.MaterialCardView
+        <androidx.appcompat.widget.Toolbar
+            android:id="@+id/toolbar"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
             android:layout_margin="16dp"
-            app:cardCornerRadius="16dp"
-            app:layout_scrollFlags="scroll|enterAlways">
-
-            <androidx.appcompat.widget.Toolbar
-                android:id="@+id/toolbar"
-                android:layout_width="match_parent"
-                android:layout_height="wrap_content"
-                android:elevation="0dp" />
-
-        </com.google.android.material.card.MaterialCardView>
+            android:background="@drawable/background_rounded_corners"
+            app:layout_scrollFlags="scroll|enterAlways"
+            app:title="@string/app_name" />
 
     </com.google.android.material.appbar.AppBarLayout>
 

app/src/main/res/layout/fragment_timer_form.xml

@@ -188,5 +188,4 @@
 
     </com.google.android.material.appbar.AppBarLayout>
 
-
 </androidx.coordinatorlayout.widget.CoordinatorLayout>