Skip to content
This repository was archived by the owner on Feb 5, 2021. It is now read-only.

Update Compose to dev12. #41

Merged
merged 1 commit into from
May 29, 2020
Merged

Conversation

zach-klippenstein
Copy link
Collaborator

@zach-klippenstein zach-klippenstein commented May 28, 2020

Release notes:

Note that ComposeWorkflow seems to break the compiler now, not sure why, but I've just disabled the module for now.

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/update-compose branch from 328ed1d to 2fa42f6 Compare May 28, 2020 16:52
@zach-klippenstein zach-klippenstein changed the title WIP Update Compose to dev12. Update Compose to dev12. May 28, 2020
@zach-klippenstein zach-klippenstein marked this pull request as ready for review May 28, 2020 16:52
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/update-compose branch 2 times, most recently from 11b67de to dd5c510 Compare May 28, 2020 17:01
@@ -70,7 +69,7 @@ internal fun placeholderViewFactory(modifier: Modifier): ViewFactory<Any> =
style = TextStyle(
textAlign = TextAlign.Center,
color = Color.White,
shadow = Shadow(blurRadius = 5.px, color = Color.Black)
shadow = Shadow(blurRadius = 5f, color = Color.Black)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Px type is being deleted, replaced with raw floats.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. But…floats? Yuck.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, I'm not a fan either.

@@ -1,7 +1,7 @@
public final class com/squareup/workflow/ui/compose/ComposeRendering {
public static final field Companion Lcom/squareup/workflow/ui/compose/ComposeRendering$Companion;
public static final fun <clinit> ()V
public fun <init> (Lkotlin/jvm/functions/Function2;)V
public fun <init> (Lkotlin/jvm/functions/Function4;)V
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These parameter count changes are due to the compiler changes that landed in dev12. See release notes for more info.

@@ -89,12 +90,8 @@ class WorkflowContainerTest {
composeRule.setContent {
WorkflowContainer(workflow, onOutput = { receivedOutputs += it }) { sendOutput ->
Column {
Clickable(onClick = { sendOutput("one") }) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clickable has been deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems in keeping with that Composable-as-Widget thought process.

Comment on lines 64 to 71
): ComposeViewFactoryRoot = object : ComposeViewFactoryRoot {
): ComposeViewFactoryRoot = ComposeViewFactoryRootImpl(wrapper)

private class ComposeViewFactoryRootImpl(
val wrapper: @Composable() (content: @Composable() () -> Unit) -> Unit
) : ComposeViewFactoryRoot {
Copy link
Collaborator Author

@zach-klippenstein zach-klippenstein May 28, 2020

Choose a reason for hiding this comment

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

This change was required to get the root to recompose if a different instance is provided in the ViewEnvironment between compose passes. See https://issuetracker.google.com/issues/157638284

Comment on lines 278 to 284
@Pivotal workflow: Workflow<PropsT, OutputT, RenderingT>,
workflow: Workflow<PropsT, OutputT, RenderingT>,
props: PropsT,
onOutput: (OutputT) -> Unit,
@Pivotal coroutineContext: CoroutineContext,
@Pivotal diagnosticListener: WorkflowDiagnosticListener?,
coroutineContext: CoroutineContext,
diagnosticListener: WorkflowDiagnosticListener?,
snapshotKey: String?
): State<RenderingT> {
): State<RenderingT> = key(workflow, coroutineContext, diagnosticListener) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Pivotal has been removed. key existed before, but is now the only way to do this.

@@ -59,12 +60,13 @@ internal fun ViewGroup.setContent(
parent: CompositionReference,
content: @Composable() () -> Unit
): Composition {
FrameManager.ensureStarted()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All the changes in this file are to ensure this code stays as close as possible to the actual private code in the runtime.

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/update-compose branch 2 times, most recently from ae4a5b8 to 1cdc2b5 Compare May 28, 2020 17:37
@zach-klippenstein zach-klippenstein requested a review from rjrjr May 28, 2020 17:37
@zach-klippenstein zach-klippenstein force-pushed the zachklipp/update-compose branch 2 times, most recently from da02136 to 8f348b0 Compare May 29, 2020 21:09
@zach-klippenstein
Copy link
Collaborator Author

Fixed broken UI tests (seems like a Compose bug on older Android versions, filed as https://issuetracker.google.com/issues/157728188) and linked an issue for the compiler crash.

@zach-klippenstein
Copy link
Collaborator Author

UI test failure seems to be due to emulator not starting.

@zach-klippenstein zach-klippenstein merged commit e5e9337 into master May 29, 2020
@zach-klippenstein zach-klippenstein deleted the zachklipp/update-compose branch May 29, 2020 22:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants