Refactor Workflow Trace Viewer to have multi window support #1417
Conversation
japplin
force-pushed
the
japplin/workflow-visualizer-title
branch
from
c1b31f7 to
921017f
Compare
| * Composable for live device trace viewing. | ||
| */ | ||
| @Composable | ||
| internal fun LiveApp( |
There was a problem hiding this comment.
THis is not used for file based? Is App used for that? We should modify the kdoc there to make that clear.
Is there much code duplication between these two composables that we can share?
There was a problem hiding this comment.
I did a cleanup pass after claude, but looking at it again with fresh eyes I see alot of room to improve still. I'll have another pass at cleaning this up.
There was a problem hiding this comment.
These are now deduplicated.
japplin
force-pushed
the
japplin/workflow-visualizer-title
branch
from
a9c7a13 to
31acfe7
Compare
japplin
force-pushed
the
japplin/workflow-visualizer-title
branch
from
1003900 to
b854808
Compare
steve-the-edwards
left a comment
steve-the-edwards
left a comment
There was a problem hiding this comment.
I did a cursory look over the Compose UI stuff. Best to have @zach-klippenstein or @handstandsam look!
|
|
||
| java-diff-utils = { module = "io.github.java-diff-utils:java-diff-utils", version.ref = "java-diff-utils" } | ||
|
|
||
| telephoto = { module = "me.saket.telephoto:zoomable", version.ref = "telephoto" } |
There was a problem hiding this comment.
Was this usage via @saket 's suggestion? :). If not, he should know!
There was a problem hiding this comment.
Nope, I was just struggling with the zooming code for hours and then remembered his library and gave it a shot. There is still room to improve our implementation by replacing the custom panning code with the zoomable library as well, I briefly tried doing that but it wasn't panning.... out, so I left the hybrid approach for now. Would be great if you were interested in contributed that improvement @saket.
There was a problem hiding this comment.
yay, love seeing more usage inside our codebase! 🎉
I briefly tried doing that but it wasn't panning
that might be because compose multiplatform has pretty limited support for zoom events on the JVM :/ last time I checked, I wasn't seeing any trackpad or mouse scroll events. I'll double-check this today. could you share instructions for opening the trace viewer?
There was a problem hiding this comment.
If you have the workflow kotlin repo cloned you can launch the trace viewer using ./gradlew :workflow-trace-viewer:run. I'm not sure if we have any sample traces in open source but you can download them from any failed test in ci I'll dm you a link.
There was a problem hiding this comment.
taking a look at this right now!
| kotlin { | ||
| jvm() | ||
|
|
||
| jvmToolchain(11) |
There was a problem hiding this comment.
Why is this needed now? Something about the published artifact?
There was a problem hiding this comment.
This was actually a byproduct of pulling in telescope.
| implementation(libs.telephoto) | ||
|
|
||
| // Add explicit Skiko dependency for current platform | ||
| implementation("org.jetbrains.skiko:skiko-awt-runtime-macos-arm64:0.9.4") |
There was a problem hiding this comment.
lets move this to libs.versions.toml otherwise we'll lose track of it.
| while (true) { | ||
| delay(3000L) | ||
| value = listDevices() | ||
| } |
There was a problem hiding this comment.
😱 . what is this about? we have to wait to get an adb connection or something?
There was a problem hiding this comment.
If so, woudl be good to comment this delay and also use a constant on that magic 3000L
There was a problem hiding this comment.
Previously the adb list was fetched when toggling live mode on, now we show the list of devices while continuously refreshing.
japplin
force-pushed
the
japplin/workflow-visualizer-title
branch
from
0a2acb4 to
84cffb0
Compare
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Adds new landing page:
Opening a trace file now launches a separate window using that trace file as the title, live view also opens a new window. This allows for comparing side by side traces.
