Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3/4 fun View.setBackHandler() replaces val View.backPressedHandler #1028

Merged
merged 1 commit into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,10 +12,10 @@ 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.control
import com.squareup.workflow1.ui.setBackHandler

@OptIn(WorkflowUiExperimentalApi::class)
data class TodoEditorScreen(
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
private var viewHandlerCount = 0
private var viewHandlerBackPressedCount = 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()")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Deprecated("Use View.backHandler()")
@Deprecated("Use View.setBackHandler()")

@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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Deprecated("Use setOrClearBackHandler")
@Deprecated("Use setBackHandler")

I see you went through some naming iterations 😄

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