Skip to content

Update templates #362

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 1 commit into from
Mar 23, 2021
Merged

Update templates #362

merged 1 commit into from
Mar 23, 2021

Conversation

tir38
Copy link
Contributor

@tir38 tir38 commented Mar 7, 2021

IMO these templates would be more helpful if they let users set types in the creation dialog, not with a later refactor.

We don't always want state/props/rendering/output to have the same name as the workflow. Nor
do we always want to create those states/props/renderings/output. Sometimes those classes already
exist (e.g. Unit, Nothing, Any).

Stateful and Stateless Workflow templates now take in optional props/state/rendering/output. If these are left blank they will be auto created

Test

  1. In Android Studio -> File -> New -> Edit File Templates....
  2. create a new template or update existing and copy/paste code from these .kt files
  3. File -> New -> select a template
  4. see generated file

StatefulWorkflow Template

optionals left blank

Screen Shot 2021-03-11 at 7 07 34 PM

generates:

package workflow.tutorial

import com.squareup.workflow1.Snapshot
import com.squareup.workflow1.StatefulWorkflow

import workflow.tutorial.MyFancyWorkflow.MyFancyProps
import workflow.tutorial.MyFancyWorkflow.MyFancyState
import workflow.tutorial.MyFancyWorkflow.MyFancyOutput
import workflow.tutorial.MyFancyWorkflow.MyFancyRendering

object MyFancyWorkflow : StatefulWorkflow<MyFancyProps, MyFancyState, MyFancyOutput, MyFancyRendering>() {

  data class MyFancyProps(
    // TODO add args
  )

  data class MyFancyState(
    // TODO add args
  )

  data class MyFancyOutput(
    // TODO add args
  )

  data class MyFancyRendering(
    // TODO add args
  )

  override fun initialState(
    props: MyFancyProps,
    snapshot: Snapshot?
  ): MyFancyState = TODO("Initialize state")

  override fun render(
    props: MyFancyProps,
    state: MyFancyState,
    context: RenderContext
  ): MyFancyRendering {
    TODO("Render")
  }

  override fun snapshotState(state: MyFancyState): Snapshot? = Snapshot.write {
    TODO("Save state")
  }
}

optionals supplied

Screen Shot 2021-03-11 at 7 09 03 PM

generates:

package workflow.tutorial

import com.squareup.workflow1.Snapshot
import com.squareup.workflow1.StatefulWorkflow

object MyFancyWorkflow : StatefulWorkflow<ExistingProps, ExistingState, Unit, ExistingScreen>() {

  override fun initialState(
    props: ExistingProps,
    snapshot: Snapshot?
  ): ExistingState = TODO("Initialize state")

  override fun render(
    props: ExistingProps,
    state: ExistingState,
    context: RenderContext
  ): ExistingScreen {
    TODO("Render")
  }

  override fun snapshotState(state: ExistingState): Snapshot? = Snapshot.write {
    TODO("Save state")
  }
}

StatelessWorkflow template

optionals left blank

Screen Shot 2021-03-11 at 7 09 57 PM

generates:

package workflow.tutorial

import com.squareup.workflow1.StatelessWorkflow

import workflow.tutorial.MyFancyWorkflow.MyFancyProps
import workflow.tutorial.MyFancyWorkflow.MyFancyOutput
import workflow.tutorial.MyFancyWorkflow.MyFancyRendering

object MyFancyWorkflow : StatelessWorkflow<MyFancyProps, MyFancyOutput, MyFancyRendering>() {

  data class MyFancyProps(
    // TODO add args   
  )

  data class MyFancyOutput(
    // TODO add args   
  )

  data class MyFancyRendering(
    // TODO add args   
  )

  override fun render(
    props: MyFancyProps,
    context: RenderContext
  ): MyFancyRendering {
    TODO("Render")
  }
}

optionals supplied

Screen Shot 2021-03-11 at 7 10 55 PM

generates:

package workflow.tutorial

import com.squareup.workflow1.StatelessWorkflow

object MyFancyWorkflow : StatelessWorkflow<ExistingProps, Nothing, ExistingScreen>() {

  override fun render(
    props: ExistingProps,
    context: RenderContext
  ): ExistingScreen {
    TODO("Render")
  }
}

LayoutRunner template

Screen Shot 2021-03-11 at 7 11 48 PM

generates:

package workflow.tutorial

import com.squareup.workflow1.ui.LayoutRunner
import com.squareup.workflow1.ui.LayoutRunner.Companion.bind
import com.squareup.workflow1.ui.ViewEnvironment
import com.squareup.workflow1.ui.ViewFactory
import com.squareup.workflow1.ui.WorkflowUiExperimentalApi

@OptIn(WorkflowUiExperimentalApi::class)
class MyFancyLayoutRunner(
  private val binding: MyExistingBindings
) : LayoutRunner<MyExistingScreen> {

  override fun showRendering(
    rendering: MyExistingScreen,
    viewEnvironment: ViewEnvironment
  ) {
    TODO("Update ViewBinding from rendering")
  }

  companion object : ViewFactory<MyExistingScreen> by bind(
    MyExistingBindings::inflate, ::MyFancyLayoutRunner
  )
}

@tir38 tir38 requested review from zach-klippenstein and a team as code owners March 7, 2021 22:58
@CLAassistant
Copy link

CLAassistant commented Mar 7, 2021

CLA assistant check
All committers have signed the CLA.

class $LayoutRunnerName(
private val binding: ${ViewBindingName}
) : LayoutRunner<${RenderingName}> {
class ${NAME}(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

LMK if there is a preference on camel case vs screaming snake case

Comment on lines -14 to -17
data class Props
data class State
data class Output
data class Rendering
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the part where it gets trickier as suggesting this pattern of internal data classes is a goal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how complex you want it to get, but there could be an option to leave the template blank for at type and then these would be generated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool. Didn't know about the internal pattern. I'll experiment with an if --> auto generate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok @steve-the-edwards I think I have it working. If you leave an "optional" field blank then it will auto generate inner class (and import)

Comment on lines 15 to 16
props: ${PROPS_TYPE},
state: ${STATE_TYPE},
Copy link
Contributor

Choose a reason for hiding this comment

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

renderProps and renderState parameters are the new API and should be maintained.

Copy link
Contributor Author

@tir38 tir38 Mar 8, 2021

Choose a reason for hiding this comment

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

👍 will revert.

My copy of render() (from com.squareup.workflow1:workflow-core-jvm:1.0.0-alpha.7) still shows props/state

renderProps: Props,
context: RenderContext
): Rendering {
props: ${PROPS_TYPE},
Copy link
Contributor

Choose a reason for hiding this comment

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

again renderProps is the new API.

We don't always want state/props/rendering/output to have the same name as the workflow. Nor
do we always want to create those states/props/renderings/output. Sometimes those classes already
exist (e.g. Unit, Nothing, Any).
@tir38 tir38 changed the title Update templates [WIP] Update templates Mar 12, 2021
@tir38 tir38 requested a review from steve-the-edwards March 12, 2021 00:16
@tir38 tir38 changed the title [WIP] Update templates Update templates Mar 12, 2021
@rjrjr
Copy link
Contributor

rjrjr commented Mar 16, 2021

LGTM. @steve-the-edwards, you good?

Copy link
Contributor

@steve-the-edwards steve-the-edwards left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks very much for this @tir38 . Very sorry for the delay in reviewing!

@steve-the-edwards steve-the-edwards merged commit dbac29f into square:main Mar 23, 2021
@tir38 tir38 deleted the tir38/update-templates branch March 23, 2021 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants