Skip to content

Simpler, safer ViewEnvironment and ViewRegistry management. #631

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

Merged
merged 2 commits into from
Jan 13, 2022
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 @@ -16,10 +16,9 @@ import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.compose.WorkflowRendering
import com.squareup.workflow1.ui.compose.renderAsState
import com.squareup.workflow1.ui.plus

private val viewRegistry = ViewRegistry(HelloBinding)

private val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
private val viewEnvironment = ViewEnvironment.EMPTY + ViewRegistry(HelloBinding)

@Composable fun App() {
MaterialTheme {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ import com.squareup.workflow1.ui.WorkflowLayout
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.compose.composeViewFactory
import com.squareup.workflow1.ui.compose.withCompositionRoot
import com.squareup.workflow1.ui.plus
import com.squareup.workflow1.ui.renderWorkflowIn
import kotlinx.coroutines.flow.StateFlow

@OptIn(WorkflowUiExperimentalApi::class)
private val viewRegistry = ViewRegistry(HelloBinding)

@OptIn(WorkflowUiExperimentalApi::class)
private val viewEnvironment =
ViewEnvironment(mapOf(ViewRegistry to viewRegistry)).withCompositionRoot { content ->
(ViewEnvironment.EMPTY + ViewRegistry(HelloBinding)).withCompositionRoot { content ->
MaterialTheme(content = content)
}

Expand All @@ -37,22 +35,22 @@ class HelloBindingActivity : AppCompatActivity() {

val model: HelloBindingModel by viewModels()
setContentView(
WorkflowLayout(this).apply {
start(
renderings = model.renderings,
environment = viewEnvironment
)
}
WorkflowLayout(this).apply {
start(
renderings = model.renderings,
environment = viewEnvironment
)
}
)
}

class HelloBindingModel(savedState: SavedStateHandle) : ViewModel() {
@OptIn(WorkflowUiExperimentalApi::class)
val renderings: StateFlow<Any> by lazy {
renderWorkflowIn(
workflow = HelloWorkflow,
scope = viewModelScope,
savedStateHandle = savedState
workflow = HelloWorkflow,
scope = viewModelScope,
savedStateHandle = savedState
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,5 @@ object HelloComposeWorkflow : ComposeWorkflow<String, Toggle>() {
@Preview(showBackground = true)
@Composable fun HelloComposeWorkflowPreview() {
val rendering by HelloComposeWorkflow.renderAsState(props = "hello", onOutput = {})
WorkflowRendering(rendering, ViewEnvironment())
WorkflowRendering(rendering, ViewEnvironment.EMPTY)
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ object InlineRenderingWorkflow : StatefulWorkflow<Unit, Int, Nothing, AndroidVie
@Preview
@Composable fun InlineRenderingWorkflowPreview() {
val rendering by InlineRenderingWorkflow.renderAsState(props = Unit, onOutput = {})
WorkflowRendering(rendering, ViewEnvironment())
WorkflowRendering(rendering, ViewEnvironment.EMPTY)
}

@OptIn(ExperimentalAnimationApi::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowLayout
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.compose.withCompositionRoot
import com.squareup.workflow1.ui.plus
import com.squareup.workflow1.ui.renderWorkflowIn
import kotlinx.coroutines.flow.StateFlow

Expand All @@ -26,7 +27,7 @@ private val viewRegistry = ViewRegistry(

@OptIn(WorkflowUiExperimentalApi::class)
private val viewEnvironment =
ViewEnvironment(mapOf(ViewRegistry to viewRegistry)).withCompositionRoot { content ->
(ViewEnvironment.EMPTY + viewRegistry).withCompositionRoot { content ->
CompositionLocalProvider(LocalBackgroundColor provides Color.Green) {
content()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@file:OptIn(WorkflowUiExperimentalApi::class)
@file:Suppress("FunctionName")
Copy link
Contributor

Choose a reason for hiding this comment

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

Which lint warning is this from and why do we have to suppress now?

Copy link
Contributor Author

@rjrjr rjrjr Jan 12, 2022

Choose a reason for hiding this comment

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

It's complaining about the capitalized function name. We don't have to suppress it, the build was green. I just hate IDE warnings.


package com.squareup.sample.compose.textinput

Expand All @@ -11,10 +12,9 @@ import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.compose.WorkflowRendering
import com.squareup.workflow1.ui.compose.renderAsState
import com.squareup.workflow1.ui.plus

private val viewRegistry = ViewRegistry(TextInputViewFactory)

private val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
private val viewEnvironment = ViewEnvironment.EMPTY + ViewRegistry(TextInputViewFactory)

@Composable fun TextInputApp() {
MaterialTheme {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package com.squareup.sample.container.overviewdetail

import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.ViewEnvironmentKey
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi

/**
* [com.squareup.workflow1.ui.ViewEnvironment] value that informs views
Expand Down Expand Up @@ -34,3 +35,7 @@ enum class OverviewDetailConfig {
override val default = None
}
}

@WorkflowUiExperimentalApi
operator fun ViewEnvironment.plus(config: OverviewDetailConfig): ViewEnvironment =
this + (OverviewDetailConfig to config)
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ class OverviewDetailContainer(view: View) : ScreenViewRunner<OverviewDetailScree
// Since we have two sibling back stacks, we need to give them each different
// SavedStateRegistry key prefixes.
val overviewViewEnvironment = viewEnvironment
.withBackStackStateKeyPrefix(OverviewBackStackKey) + (OverviewDetailConfig to Overview)
.withBackStackStateKeyPrefix(OverviewBackStackKey) + Overview
overviewStub!!.show(rendering.overviewRendering, overviewViewEnvironment)
rendering.detailRendering
?.let { detail ->
detailStub!!.actual.visibility = VISIBLE
detailStub.show(
detail,
viewEnvironment + (OverviewDetailConfig to Detail)
viewEnvironment + Detail
)
}
?: run {
Expand All @@ -81,7 +81,7 @@ class OverviewDetailContainer(view: View) : ScreenViewRunner<OverviewDetailScree
?.let { rendering.overviewRendering + it }
?: rendering.overviewRendering

stub.show(combined, viewEnvironment + (OverviewDetailConfig to Single))
stub.show(combined, viewEnvironment + Single)
}

companion object : ScreenViewFactory<OverviewDetailScreen> by ScreenViewRunner.bind(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.ViewFactory
import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.plus
import kotlin.reflect.KClass

/**
Expand All @@ -30,7 +31,7 @@ import kotlin.reflect.KClass
PreviewViewRegistry(mainFactory, placeholderViewFactory(placeholderModifier))
}
return remember(viewRegistry, viewEnvironmentUpdater) {
ViewEnvironment(mapOf(ViewRegistry to viewRegistry)).let { environment ->
(ViewEnvironment.EMPTY + viewRegistry).let { environment ->
// Give the preview a chance to add its own elements to the ViewEnvironment.
viewEnvironmentUpdater?.let { it(environment) } ?: environment
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.WorkflowViewStub
import com.squareup.workflow1.ui.internal.test.IdleAfterTestRule
import com.squareup.workflow1.ui.plus
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
Expand All @@ -39,8 +40,7 @@ internal class ComposeViewFactoryTest {
val viewFactory = composeViewFactory<Unit> { _, _ ->
BasicText("Hello, world!")
}
val viewRegistry = ViewRegistry(viewFactory)
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
val viewEnvironment = ViewEnvironment.EMPTY + ViewRegistry(viewFactory)

composeRule.setContent {
AndroidView(::RootView) {
Expand All @@ -55,8 +55,7 @@ internal class ComposeViewFactoryTest {
val viewFactory = composeViewFactory<String> { rendering, _ ->
BasicText(rendering, Modifier.testTag("text"))
}
val viewRegistry = ViewRegistry(viewFactory)
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to viewRegistry))
val viewEnvironment = ViewEnvironment.EMPTY + ViewRegistry(viewFactory)
var rendering by mutableStateOf("hello")

composeRule.setContent {
Expand All @@ -82,12 +81,7 @@ internal class ComposeViewFactoryTest {
}
val viewRegistry = ViewRegistry(viewFactory)
var viewEnvironment by mutableStateOf(
ViewEnvironment(
mapOf(
ViewRegistry to viewRegistry,
testEnvironmentKey to "hello"
)
)
ViewEnvironment.EMPTY + viewRegistry + (testEnvironmentKey to "hello")
)

composeRule.setContent {
Expand All @@ -104,7 +98,7 @@ internal class ComposeViewFactoryTest {

@Test fun wrapsFactoryWithRoot() {
val wrapperText = mutableStateOf("one")
val viewEnvironment = ViewEnvironment(mapOf(ViewRegistry to ViewRegistry(TestFactory)))
val viewEnvironment = ViewEnvironment.EMPTY + ViewRegistry(TestFactory)
.withCompositionRoot { content ->
Column {
BasicText(wrapperText.value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,20 @@ import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import com.google.common.truth.Truth.assertThat
import com.squareup.workflow1.ui.AndroidViewRendering
import com.squareup.workflow1.ui.asScreen
import com.squareup.workflow1.ui.Compatible
import com.squareup.workflow1.ui.NamedScreen
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.ViewFactory
import com.squareup.workflow1.ui.ViewRegistry
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
import com.squareup.workflow1.ui.asScreen
import com.squareup.workflow1.ui.bindShowRendering
import com.squareup.workflow1.ui.container.BackStackScreen
import com.squareup.workflow1.ui.internal.test.IdleAfterTestRule
import com.squareup.workflow1.ui.internal.test.WorkflowUiTestActivity
import com.squareup.workflow1.ui.modal.HasModals
import com.squareup.workflow1.ui.modal.ModalViewContainer
import com.squareup.workflow1.ui.plus
import org.junit.Before
import org.junit.Rule
import org.junit.Test
Expand All @@ -55,14 +56,8 @@ internal class ComposeViewTreeIntegrationTest {

@Before fun setUp() {
scenario.onActivity {
it.viewEnvironment = ViewEnvironment(
mapOf(
ViewRegistry to ViewRegistry(
ModalViewContainer.binding<TestModalScreen>(),
NoTransitionBackStackContainer,
)
)
)
it.viewEnvironment = ViewEnvironment.EMPTY +
ViewRegistry(ModalViewContainer.binding<TestModalScreen>(), NoTransitionBackStackContainer)
}
}

Expand Down
Loading