Fix more PR comments

Author: wenli-cai

workflow-trace-viewer/README.md

@@ -24,22 +24,22 @@ click to expand/collapse a specific node's children.
 
 <img src="https://github.com/square/workflow-kotlin/raw/wenli/improve-visualizer/workflow-trace-viewer/docs/demo.gif" width="320" alt="Demo" />
 
-#### File Mode
+### File Mode
 
 Once a file of the live data is saved, it can easily be uploaded to retrace the steps taken during
 the live session. Currently, text/json files that are saved from recordings only contain raw data,
 meaning it is simply a list of lists of node renderings.
 
 <img src="https://github.com/square/workflow-kotlin/raw/wenli/improve-visualizer/workflow-trace-viewer/docs/file_mode.gif" width="320" alt="File Mode" />
 
-#### Live Mode
+### Live Mode
 
 By hitting the bottom switch, you are able to toggle to live stream mode, where data is directly
 pulled from the emulator into the visualizer. To do so:
 
-1. Start the app (on any device)
-2. Start the app, and toggle the switch to enter Live mode
-3. Select the desired device
+- Start the app (on any device)
+- Start the app, and toggle the switch to enter Live mode
+- Select the desired device
 
 Once in Live mode, frames will appear as you interact with the app. You may also save the current
 data into a file saved in `~/Downloads` to be used later (this action will take some time, so it may
@@ -58,10 +58,10 @@ A connection can only happen once. There is currently no support for a recording
 due to the fact that an open socket will consume all render pass data when a connection begins. To
 restart the recording:
 
-1. (optional) Save the current trace
-2. Switch out of Live mode
-3. Restart the app
-4. Switch back to Live mode, and the
+- (optional) Save the current trace
+- Switch out of Live mode
+- Restart the app
+- Switch back to Live mode, and the
 
 ### Terminology
 
@@ -83,4 +83,4 @@ on Kotlin and KMP projects. This simplified the development process of allowing
 ## Future
 
 This app can be integrated into the process of anyone working with Workflow, so it's highly
-encouraged for anyone to make improvements that makes their life a little easier using this app. 
+encouraged for anyone to make improvements that makes their life a little easier using this app.

workflow-trace-viewer/docs/demo.gif

@@ -9,6 +9,7 @@ import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableFloatStateOf
 import androidx.compose.runtime.mutableIntStateOf
+import androidx.compose.runtime.mutableStateListOf
 import androidx.compose.runtime.mutableStateMapOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
@@ -18,11 +19,9 @@ import androidx.compose.runtime.snapshots.SnapshotStateMap
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.geometry.Offset
-import androidx.compose.ui.layout.onGloballyPositioned
 import androidx.compose.ui.layout.onSizeChanged
 import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.dp
-import androidx.compose.ui.window.WindowPosition.PlatformDefault.x
 import com.squareup.workflow1.traceviewer.model.Node
 import com.squareup.workflow1.traceviewer.model.NodeUpdate
 import com.squareup.workflow1.traceviewer.ui.RightInfoPanel
@@ -49,7 +48,7 @@ internal fun App(
   var rawRenderPass by remember { mutableStateOf("") }
   var frameIndex by remember { mutableIntStateOf(0) }
   val sandboxState = remember { SandboxState() }
-  val nodeLocations = remember { mutableListOf<SnapshotStateMap<Node, Offset>>() }
+  val nodeLocations = remember { mutableStateListOf<SnapshotStateMap<Node, Offset>>() }
 
   // Default to File mode, and can be toggled to be in Live mode.
   var active by remember { mutableStateOf(false) }
@@ -110,7 +109,9 @@ internal fun App(
       horizontalAlignment = Alignment.CenterHorizontally
     ) {
       if (active) {
-        // Since we can jump from frame to frame, we fill in the map during each recomposition
+        // Frames that appear in composition may not happen sequentially, so when the current frame
+        // locations is null, that means we've skipped frames and need to fill all the intermediate
+        // ones. e.g. Frame 1 to Frame 10
         if (nodeLocations.getOrNull(frameInd) == null) {
           // frameSize has not been updated yet, so on the first frame, frameSize = nodeLocations.size = 0,
           // and it will append a new map

workflow-trace-viewer/docs/file_mode.gif

@@ -21,7 +21,7 @@ internal data class NodeUpdate(
         past == null -> NodeState.NEW
         current.props != past.props -> NodeState.PROPS_CHANGED
         current.state != past.state -> NodeState.STATE_CHANGED
-        else -> NodeState.CHILDREN_CHANGED
+        else -> NodeState.RENDERED
       }
 
       return NodeUpdate(current, past, state)
@@ -33,6 +33,6 @@ internal enum class NodeState(val color: Color) {
   NEW(Color(0x804CAF50)), // green
   STATE_CHANGED(Color(0xFFE57373)), // red
   PROPS_CHANGED(Color(0xFFFF8A65)), // orange
-  CHILDREN_CHANGED(Color(0x802196F3)), // blue
+  RENDERED(Color(0x802196F3)), // blue
   UNCHANGED(Color.LightGray.copy(alpha = 0.3f)),
 }

workflow-trace-viewer/docs/live_mode.gif

@@ -62,7 +62,6 @@ internal fun RightInfoPanel(
         .clip(CircleShape)
         .background(Color.White)
         .padding(8.dp)
-        .size(40.dp)
         .align(Alignment.Top)
     ) {
       Icon(

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/App.kt

@@ -306,8 +306,10 @@ private fun DrawNode(
   )
 
   Box {
-    Box(
+    Column(
+      horizontalAlignment = Alignment.CenterHorizontally,
       modifier = Modifier
+        .wrapContentSize()
         .hoverable(interactionSource)
         .background(nodeUpdate.state.color)
         .clickable {
@@ -328,21 +330,16 @@ private fun DrawNode(
           storeNodeLocation(node, offsetToCenter)
         }
     ) {
-      Column(
-        horizontalAlignment = Alignment.CenterHorizontally,
-        modifier = Modifier.wrapContentSize()
+      Row(
+        verticalAlignment = Alignment.CenterVertically,
+        horizontalArrangement = Arrangement.spacedBy(4.dp)
       ) {
-        Row(
-          verticalAlignment = Alignment.CenterVertically,
-          horizontalArrangement = Arrangement.spacedBy(4.dp)
-        ) {
-          if (node.children.isNotEmpty()) {
-            Text(text = if (isExpanded) "▼" else "▶")
-          }
-          Text(text = node.name)
+        if (node.children.isNotEmpty()) {
+          Text(text = if (isExpanded) "▼" else "▶")
         }
-        Text(text = "ID: ${node.id}")
+        Text(text = node.name)
       }
+      Text(text = "ID: ${node.id}")
     }
 
     if (isHovered) {

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/model/NodeUpdate.kt

@@ -1,6 +1,7 @@
 package com.squareup.workflow1.traceviewer.ui.control
 
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.widthIn
 import androidx.compose.foundation.shape.CircleShape
 import androidx.compose.material.Button
 import androidx.compose.material.ButtonDefaults.buttonColors
@@ -24,29 +25,34 @@ internal fun FileDump(
   trace: String,
   modifier: Modifier = Modifier
 ) {
+  var filePath by remember { mutableStateOf("") }
   var clicked by remember { mutableStateOf(false) }
   Button(
-    modifier = modifier.padding(16.dp),
+    modifier = modifier
+      .padding(16.dp)
+      .widthIn(max = 300.dp),
     shape = CircleShape,
     colors = buttonColors(Color.Black),
     onClick = {
       clicked = true
-      writeToFile(trace)
+      filePath = writeToFile(trace)
     }
   ) {
     val text = if (clicked) {
-      "Trace saved to Downloads"
+      "Trace saved to $filePath"
     } else {
       "Save trace to file"
     }
     Text(
       text = text,
-      color = Color.White
+      color = Color.White,
+      maxLines = 3,
+      softWrap = true
     )
   }
 }
 
-private fun writeToFile(trace: String) {
+private fun writeToFile(trace: String): String{
   val timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd_HHmmss"))
   val home = System.getProperty("user.home")
   val path = "$home/Downloads/workflow-trace_$timestamp.json".toPath()
@@ -58,4 +64,6 @@ private fun writeToFile(trace: String) {
       bufferedSink.writeUtf8("]")
     }
   }
+
+  return path.toString()
 }