Skip to content

Conversation

@saket
Copy link
Collaborator

@saket saket commented Sep 5, 2025

Context: #1417 (comment)

This PR removes all custom pointer handlers in SandboxBackground() so that Modifier.zoomable() can take care of both panning and zooming.

One change to call out is that zooming with the mouse scroll now behaves differently. Previously, you were using scroll alone, but telephoto uses Option + scroll by default to match the behavior of other macOS apps. If you prefer the old behavior, you can change this if you want by writing a custom HardwareShortcutsSpec

It might be a good idea to display all shortcuts somewhere in the UI. Here's what telephoto uses: 
https://saket.github.io/telephoto/zoomable/#keyboard-shortcuts

Your app actually helped uncover a few bugs in telephoto, which historically hasn't been well optimized for non-Android targets. Some worth mentioning:

  • Zoom using keyboard and mouse shortcuts is currently broken because telephoto is miscalculating zoom factors. I'll push a fix soon.

  • Overzoom doesn't look great right now. I'll disable it for hardware shortcuts in a future release.

CleanShot.2025-09-05.at.15.57.01.mp4

@saket saket requested review from a team and zach-klippenstein as code owners September 5, 2025 21:24
@CLAassistant
Copy link

CLAassistant commented Sep 5, 2025

CLA assistant check
All committers have signed the CLA.

rememberZoomableState(
zoomSpec = ZoomSpec(maxZoomFactor = 1f)
).also {
it.contentScale = ContentScale.Fit
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It might be worth exploring ContentScale.Crop for large traces

@steve-the-edwards
Copy link
Contributor

@saket sorry I need to take a look to see why the appId is failing to get created for the Github App token that we use to manage auto-fixes. (I thought I fixed it so that these shards would still run even without write perms)

I did add you to the team list for this repo so I thought that would in the mean time workaround the write perms issue, but it seems it is not 😕 .

@japplin japplin force-pushed the japplin/workflow-visualizer-title branch from 0a2acb4 to 84cffb0 Compare September 9, 2025 14:42
@japplin japplin deleted the branch square:japplin/workflow-visualizer-title September 9, 2025 15:30
@japplin japplin closed this Sep 9, 2025
@japplin
Copy link
Contributor

japplin commented Sep 9, 2025

I did not expect merging my other branch to close this branch 😬 . I'll try to rebase the branch and reopen against main.

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