1
0
Fork 0

No issue: improve allowUndo

This patch fixes a few issues:
- it was an extension on a CoroutineScope, but that was quite misleading
since the Main dispatcher would be always used regardless of what dispatcher
the owning CoroutineScope was configured with.
- timing was reliant on exact value of the undocumented Snackbar.LENGTH_LONG duration
- coroutine cancellation relied on cooperation of the 'operation' suspend function,
which we can't depend on

New 'allowUndo' fully controls its timing, doesn't imply a dispatcher to its consumers,
and doesn't rely on cooperation of passed-in suspend blocks for cancellation to work.
master
Grisha Kruglov 2019-05-29 17:14:51 -07:00 committed by Grisha Kruglov
parent 3560e1793c
commit 5f42a65c2a
5 changed files with 73 additions and 41 deletions

View File

@ -56,6 +56,7 @@ class FenixSnackbar private constructor(
companion object {
const val LENGTH_LONG = Snackbar.LENGTH_LONG
const val LENGTH_SHORT = Snackbar.LENGTH_SHORT
const val LENGTH_INDEFINITE = Snackbar.LENGTH_INDEFINITE
private const val minTextSize = 12
private const val maxTextSize = 18

View File

@ -1,35 +0,0 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.ext
import android.view.View
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers.Main
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import org.mozilla.fenix.components.FenixSnackbar
fun CoroutineScope.allowUndo(
view: View,
message: String,
undoActionTitle: String,
onCancel: suspend () -> Unit = {},
operation: suspend () -> Unit
) {
val undoJob = launch(Main) {
delay(UNDO_DELAY)
operation.invoke()
}
FenixSnackbar.make(view, FenixSnackbar.LENGTH_LONG)
.setText(message)
.setAction(undoActionTitle) {
undoJob.cancel()
launch(Main) {
onCancel.invoke()
}
}.show()
}
internal const val UNDO_DELAY = 3000L

View File

@ -54,7 +54,6 @@ import org.mozilla.fenix.R
import org.mozilla.fenix.collections.CreateCollectionViewModel
import org.mozilla.fenix.collections.SaveCollectionStep
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.allowUndo
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.urlToTrimmedHost
import org.mozilla.fenix.home.sessioncontrol.CollectionAction
@ -76,6 +75,7 @@ import org.mozilla.fenix.mvi.getManagedEmitter
import org.mozilla.fenix.onboarding.FenixOnboarding
import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.share.ShareTab
import org.mozilla.fenix.utils.allowUndo
import kotlin.coroutines.CoroutineContext
import kotlin.math.roundToInt
@ -552,7 +552,7 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
useCases.removeAllTabsOfType.invoke(isPrivate)
}
CoroutineScope(Dispatchers.Main).allowUndo(
allowUndo(
view!!, getString(R.string.snackbar_tabs_deleted),
getString(R.string.snackbar_deleted_undo), {
deleteAllSessionsJob = null
@ -594,7 +594,7 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
}
}
CoroutineScope(Dispatchers.Main).allowUndo(
allowUndo(
view!!, getString(R.string.snackbar_tab_deleted),
getString(R.string.snackbar_deleted_undo), {
deleteSessionJob = null

View File

@ -44,12 +44,12 @@ import org.mozilla.fenix.R
import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.allowUndo
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.urlToTrimmedHost
import org.mozilla.fenix.mvi.ActionBusFactory
import org.mozilla.fenix.mvi.getAutoDisposeObservable
import org.mozilla.fenix.mvi.getManagedEmitter
import org.mozilla.fenix.utils.allowUndo
import kotlin.coroutines.CoroutineContext
@SuppressWarnings("TooManyFunctions", "LargeClass")
@ -261,7 +261,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
getManagedEmitter<BookmarkChange>()
.onNext(BookmarkChange.Change(currentRoot - it.item.guid))
CoroutineScope(IO).allowUndo(
allowUndo(
view!!,
getString(R.string.bookmark_deletion_snackbar_message, it.item.url.urlToTrimmedHost()),
getString(R.string.bookmark_undo_deletion), { refreshBookmarks() }
@ -335,7 +335,7 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
val selectedBookmarks = getSelectedBookmarks()
getManagedEmitter<BookmarkChange>().onNext(BookmarkChange.Change(currentRoot - selectedBookmarks))
CoroutineScope(IO).allowUndo(
allowUndo(
view!!, getString(R.string.bookmark_deletion_multiple_snackbar_message),
getString(R.string.bookmark_undo_deletion), { refreshBookmarks() }
) {

View File

@ -0,0 +1,66 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.utils
import android.view.View
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import org.mozilla.fenix.components.FenixSnackbar
import java.util.concurrent.atomic.AtomicBoolean
internal const val UNDO_DELAY = 3000L
/**
* Runs [operation] after giving user time (see [UNDO_DELAY]) to cancel it.
* In case of cancellation, [onCancel] is executed.
*
* Execution of suspend blocks happens on [Dispatchers.Main].
*
* @param view A [View] used to determine a parent for the [FenixSnackbar].
* @param message A message displayed as part of [FenixSnackbar].
* @param undoActionTitle Label for the action associated with the [FenixSnackbar].
* @param onCancel A suspend block to execute in case of cancellation.
* @param operation A suspend block to execute if user doesn't cancel via the displayed [FenixSnackbar].
*/
fun allowUndo(
view: View,
message: String,
undoActionTitle: String,
onCancel: suspend () -> Unit = {},
operation: suspend () -> Unit
) {
val mainScope = CoroutineScope(Dispatchers.Main)
// By using an AtomicBoolean, we achieve memory effects of reading and
// writing a volatile variable.
val requestedUndo = AtomicBoolean(false)
// Launch an indefinite snackbar.
val snackbar = FenixSnackbar
.make(view, FenixSnackbar.LENGTH_INDEFINITE)
.setText(message)
.setAction(undoActionTitle) {
requestedUndo.set(true)
mainScope.launch {
onCancel.invoke()
}
}
// If user engages with the snackbar, it'll get automatically dismissed.
snackbar.show()
// Wait a bit, and if user didn't request cancellation, proceed with
// requested operation and hide the snackbar.
mainScope.launch {
delay(UNDO_DELAY)
if (!requestedUndo.get()) {
snackbar.dismiss()
operation.invoke()
}
}
}