Skip to content

Commit

Permalink
fun View.setBackHandler() replaces val View.backPressedHandler
Browse files Browse the repository at this point in the history
`val View.backPressedHandler` is overly complicated and kind of naive. The replacement, `fun View.setBackHandler()`, carefully echoes the API and behavior of [`@Composable fun BackHandler()`](https://developer.android.com/reference/kotlin/androidx/activity/compose/package-summary#BackHandler(kotlin.Boolean,kotlin.Function0)). We also provide a single argument overload that take a nullable handler function, and sets `enabled` to true for non-`null` handlers, false for `null`.

The biggest change in implementation is that we now use the preferred `OnBackPressedDispatcher.addCallback(LifecycleOwner, OnBackPressedCallback)` overload, which should allow AndroidX to do all the lifecycle bookkeeping for us -- taking advantage of all the hard work we've done to make `WorkflowLifecycleOwner` behave correctly.

This work revealed a problem where there was no `WorkflowLifecycleOwner` in place in time for the first update of the content view in `ScreenOverlayDialogFactory`, which prevented us from repeating that mistake in the new `ComponentDialog.setContent()` extension function introduced two PRs up. So that's nice.
  • Loading branch information
rjrjr committed Jun 14, 2023
1 parent 35ce30e commit f4b8dc7
Show file tree
Hide file tree
Showing 22 changed files with 222 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ class HelloBackButtonEspressoTest {
.around(IdlingDispatcherRule)

@Test fun wrappedTakesPrecedence() {
// The root workflow (AreYouSureWorkflow) wraps its child renderings
// (instances of HelloBackButtonScreen) in its own BackButtonScreen,
// which shows the "Are you sure" dialog.
// That should only be in effect on the Able screen, which sets no backHandler of its
// own. The Baker and Charlie screens set their own backHandlers,
// which should take precedence over the root one. Thus, we should
// be able to push to Charlie and pop all the way back to Able
// without seeing the "Are you sure" dialog.
onView(withId(R.id.hello_message)).apply {
check(matches(withText("Able")))
perform(click())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ object AreYouSureWorkflow :
BackButtonScreen(ableBakerCharlie) {
// While we always provide a back button handler, by default the view code
// associated with BackButtonScreen ignores ours if the view created for the
// wrapped rendering sets a handler of its own. (Set BackButtonScreen.override
// wrapped rendering sets a handler of its own. (Set BackButtonScreen.shadow
// to change this precedence.)
context.actionSink.send(maybeQuit)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import com.squareup.sample.hellobackbutton.databinding.HelloBackButtonLayoutBind
import com.squareup.workflow1.ui.AndroidScreen
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
data class HelloBackButtonScreen(
Expand All @@ -18,6 +18,6 @@ data class HelloBackButtonScreen(
ScreenViewFactory.fromViewBinding(HelloBackButtonLayoutBinding::inflate) { rendering, _ ->
helloMessage.text = rendering.message
helloMessage.setOnClickListener { rendering.onClick() }
helloMessage.backPressedHandler = rendering.onBackPressed
helloMessage.setBackHandler(rendering.onBackPressed)
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.squareup.sample.poetry

import android.annotation.SuppressLint
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
Expand All @@ -15,9 +16,9 @@ import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewRunner
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.container.BackStackConfig
import com.squareup.workflow1.ui.container.BackStackConfig.Other
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
data class StanzaListScreen(
Expand All @@ -40,6 +41,7 @@ private class StanzaListLayoutRunner(view: View) : ScreenViewRunner<StanzaListSc

private val adapter = Adapter()

@SuppressLint("NotifyDataSetChanged")
override fun showRendering(
rendering: StanzaListScreen,
environment: ViewEnvironment
Expand All @@ -53,10 +55,10 @@ private class StanzaListLayoutRunner(view: View) : ScreenViewRunner<StanzaListSc

if (environment[BackStackConfig] == Other) {
toolbar.setNavigationOnClickListener { rendering.onExit() }
toolbar.backPressedHandler = rendering.onExit
toolbar.setBackHandler(rendering.onExit)
} else {
toolbar.navigationIcon = null
toolbar.backPressedHandler = null
toolbar.setBackHandler {}
}

if (rendering.selection >= 0) recyclerView.scrollToPosition(rendering.selection)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewRunner
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.container.BackStackConfig
import com.squareup.workflow1.ui.container.BackStackConfig.None
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
data class StanzaScreen(
Expand Down Expand Up @@ -93,8 +93,10 @@ private class StanzaLayoutRunner(private val view: View) : ScreenViewRunner<Stan
toolbar.navigationIcon = null
}

view.backPressedHandler = rendering.onGoBack
val goBackOrUp = rendering.onGoBack
?: rendering.onGoUp.takeIf { environment[OverviewDetailConfig] != Detail }

view.setBackHandler(goBackOrUp)
}

private fun TextView.setTabulatedText(lines: List<String>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import com.squareup.workflow1.ui.ScreenViewRunner
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.WorkflowViewStub
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler
import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.ExperimentalTime
Expand Down Expand Up @@ -44,7 +44,7 @@ class ShakeableTimeMachineLayoutRunner(
environment: ViewEnvironment
) {
// Only handle back presses explicitly if in playback mode.
view.backPressedHandler = rendering.onResumeRecording.takeUnless { rendering.recording }
view.setBackHandler(rendering.onResumeRecording.takeUnless { rendering.recording })

seek.max = rendering.totalDuration.toProgressInt()
seek.progress = rendering.playbackPosition.toProgressInt()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.sample.tictactoe.databinding.LoginLayoutBinding
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewFactory.Companion.fromViewBinding
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
internal val LoginViewFactory: ScreenViewFactory<LoginScreen> =
Expand All @@ -15,5 +15,5 @@ internal val LoginViewFactory: ScreenViewFactory<LoginScreen> =
rendering.onLogin(loginEmail.text.toString(), loginPassword.text.toString())
}

root.backPressedHandler = { rendering.onCancel() }
root.setBackHandler(rendering.onCancel)
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import com.squareup.sample.tictactoe.databinding.SecondFactorLayoutBinding
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewFactory.Companion.fromViewBinding
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
internal val SecondFactorViewFactory: ScreenViewFactory<SecondFactorScreen> =
fromViewBinding(SecondFactorLayoutBinding::inflate) { rendering, _ ->
root.backPressedHandler = { rendering.onCancel() }
root.setBackHandler(rendering.onCancel)
secondFactorToolbar.setNavigationOnClickListener { rendering.onCancel() }

secondFactorErrorMessage.text = rendering.errorMessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import com.squareup.workflow1.ui.ScreenViewFactory.Companion.fromViewBinding
import com.squareup.workflow1.ui.ScreenViewRunner
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
internal class GameOverLayoutRunner(
Expand All @@ -40,14 +40,15 @@ internal class GameOverLayoutRunner(
rendering.onPlayAgain()
true
}
binding.root.backPressedHandler = { rendering.onExit() }
binding.root.setBackHandler(rendering.onExit)

when (rendering.endGameState.syncState) {
SAVING -> {
saveItem.isEnabled = false
saveItem.title = "saving…"
saveItem.setOnMenuItemClickListener(null)
}

SAVE_FAILED -> {
saveItem.isEnabled = true
saveItem.title = "Unsaved"
Expand All @@ -56,6 +57,7 @@ internal class GameOverLayoutRunner(
true
}
}

SAVED -> {
saveItem.isVisible = false
saveItem.setOnMenuItemClickListener(null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import com.squareup.sample.tictactoe.databinding.GamePlayLayoutBinding
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewFactory.Companion.fromViewBinding
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
internal val GamePlayViewFactory: ScreenViewFactory<GamePlayScreen> =
Expand All @@ -15,7 +15,7 @@ internal val GamePlayViewFactory: ScreenViewFactory<GamePlayScreen> =
rendering.gameState.board.render(gamePlayBoard.root)

setCellClickListeners(gamePlayBoard.root, rendering.gameState, rendering.onClick)
root.backPressedHandler = rendering.onQuit
root.setBackHandler(rendering.onQuit)
}

private fun setCellClickListeners(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.squareup.sample.tictactoe.databinding.NewGameLayoutBinding
import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewFactory.Companion.fromViewBinding
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
internal val NewGameViewFactory: ScreenViewFactory<NewGameScreen> =
Expand All @@ -16,5 +16,5 @@ internal val NewGameViewFactory: ScreenViewFactory<NewGameScreen> =
rendering.onStartGame(playerX.text.toString(), playerO.text.toString())
}

root.backPressedHandler = { rendering.onCancel() }
root.setBackHandler(rendering.onCancel)
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import com.squareup.workflow1.ui.ScreenViewFactory
import com.squareup.workflow1.ui.ScreenViewRunner
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.backPressedHandler
import com.squareup.workflow1.ui.setBackHandler
import com.squareup.workflow1.ui.container.BackStackConfig
import com.squareup.workflow1.ui.container.BackStackConfig.Other
import com.squareup.workflow1.ui.control
Expand Down Expand Up @@ -62,7 +62,7 @@ private class Runner(

if (environment[BackStackConfig] == Other) {
todoEditorToolbar.setNavigationOnClickListener { rendering.onGoBackClicked() }
root.backPressedHandler = { rendering.onGoBackClicked() }
root.setBackHandler(rendering.onGoBackClicked)
} else {
todoEditorToolbar.navigationIcon = null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import com.squareup.workflow1.ui.ScreenViewHolder
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport.onBackPressedDispatcherOwner
import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport.onBackPressedDispatcherOwnerOrNull
import com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner
import com.squareup.workflow1.ui.asScreen
import com.squareup.workflow1.ui.bindShowRendering
import com.squareup.workflow1.ui.container.BackButtonScreen
Expand Down Expand Up @@ -79,7 +81,17 @@ public open class ModalViewContainer @JvmOverloads constructor(
initialEnvironment = initialViewEnvironment,
contextForNewView = this.context,
container = this
)
) { view, doStart ->
// Note that we never call destroyOnDetach for this owner. That's okay because
// ModalContainer.update puts one in place above us on the decor view,
// and cleans it up. It's in place by the time we attach to the window, and
// so becomes our parent.
WorkflowLifecycleOwner.installOn(
view,
initialViewEnvironment.onBackPressedDispatcherOwner(view)
)
doStart()
}

return buildDialogForView(viewHolder.view)
.apply {
Expand Down
6 changes: 6 additions & 0 deletions workflow-ui/core-android/api/core-android.api
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ public final class com/squareup/workflow1/ui/TextControllerControlEditTextKt {
public static final fun control (Lcom/squareup/workflow1/ui/TextController;Landroid/widget/EditText;)V
}

public final class com/squareup/workflow1/ui/ViewBackHandlerKt {
public static final fun setBackHandler (Landroid/view/View;Lkotlin/jvm/functions/Function0;)V
public static final fun setBackHandler (Landroid/view/View;ZLkotlin/jvm/functions/Function0;)V
public static synthetic fun setBackHandler$default (Landroid/view/View;ZLkotlin/jvm/functions/Function0;ILjava/lang/Object;)V
}

public final class com/squareup/workflow1/ui/ViewBindingScreenViewFactory : com/squareup/workflow1/ui/ScreenViewFactory {
public fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function3;Lkotlin/jvm/functions/Function1;)V
public fun buildView (Lcom/squareup/workflow1/ui/Screen;Lcom/squareup/workflow1/ui/ViewEnvironment;Landroid/content/Context;Landroid/view/ViewGroup;)Lcom/squareup/workflow1/ui/ScreenViewHolder;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@file:Suppress("DEPRECATION")

package com.squareup.workflow1.ui

import android.view.View
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.squareup.workflow1.ui

import android.view.View
import androidx.activity.ComponentActivity
import androidx.test.ext.junit.rules.ActivityScenarioRule
import com.google.common.truth.Truth.assertThat
import com.squareup.workflow1.ui.androidx.WorkflowLifecycleOwner
import com.squareup.workflow1.ui.internal.test.IdlingDispatcherRule
import leakcanary.DetectLeaksAfterTestSuccess
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain

@OptIn(WorkflowUiExperimentalApi::class)
internal class ViewBackHandlerTest {
private val scenarioRule = ActivityScenarioRule(ComponentActivity::class.java)
private val scenario get() = scenarioRule.scenario

@get:Rule val rules: RuleChain = RuleChain.outerRule(DetectLeaksAfterTestSuccess())
.around(scenarioRule)
.around(IdlingDispatcherRule)

private var viewHandlerCount = 0
private fun viewBackHandler() {
viewHandlerCount++
}

@Test fun itWorksWhenHandlerIsAddedBeforeAttach() {
scenario.onActivity { activity ->
val view = View(activity)
WorkflowLifecycleOwner.installOn(view, activity)
view.setBackHandler { viewBackHandler() }

activity.setContentView(view)
assertThat(viewHandlerCount).isEqualTo(0)

activity.onBackPressedDispatcher.onBackPressed()
assertThat(viewHandlerCount).isEqualTo(1)
}
}

@Test fun itWorksWhenHandlerIsAddedAfterAttach() {
scenario.onActivity { activity ->
val view = View(activity)
activity.setContentView(view)

view.setBackHandler { viewBackHandler() }
assertThat(viewHandlerCount).isEqualTo(0)

activity.onBackPressedDispatcher.onBackPressed()
assertThat(viewHandlerCount).isEqualTo(1)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.squareup.workflow1.ui.androidx.WorkflowAndroidXSupport.onBackPressedD
* A function passed to [View.backPressedHandler], to be called if the back
* button is pressed while that view is attached to a window.
*/
@Deprecated("Use View.backHandler()")
@WorkflowUiExperimentalApi
public typealias BackPressHandler = () -> Unit

Expand All @@ -24,7 +25,9 @@ public typealias BackPressHandler = () -> Unit
* That means that this is a last-registered-first-served mechanism, and that it is
* compatible with Compose back button handling.
*/
@Suppress("DEPRECATION")
@WorkflowUiExperimentalApi
@Deprecated("Use setOrClearBackHandler")
public var View.backPressedHandler: BackPressHandler?
get() = observerOrNull?.handler
set(value) {
Expand All @@ -37,9 +40,9 @@ public var View.backPressedHandler: BackPressHandler?

@WorkflowUiExperimentalApi
private var View.observerOrNull: AttachStateAndLifecycleObserver?
get() = getTag(R.id.view_back_handler) as AttachStateAndLifecycleObserver?
get() = getTag(R.id.view_deprecated_back_handler) as AttachStateAndLifecycleObserver?
set(value) {
setTag(R.id.view_back_handler, value)
setTag(R.id.view_deprecated_back_handler, value)
}

/**
Expand Down Expand Up @@ -79,7 +82,7 @@ private var View.observerOrNull: AttachStateAndLifecycleObserver?
@WorkflowUiExperimentalApi
private class AttachStateAndLifecycleObserver(
private val view: View,
val handler: BackPressHandler
@Suppress("DEPRECATION") val handler: BackPressHandler
) : OnAttachStateChangeListener, DefaultLifecycleObserver {
private val onBackPressedCallback = NullableOnBackPressedCallback()
private var lifecycleOrNull: Lifecycle? = null
Expand Down Expand Up @@ -130,6 +133,7 @@ private class AttachStateAndLifecycleObserver(

@WorkflowUiExperimentalApi
internal class NullableOnBackPressedCallback : OnBackPressedCallback(false) {
@Suppress("DEPRECATION")
var handlerOrNull: BackPressHandler? = null

override fun handleOnBackPressed() {
Expand Down
Loading

0 comments on commit f4b8dc7

Please sign in to comment.