Skip to content
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

Add ability to pinch to zoom in Previewer area using trackpad #1196

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lyncasterc
Copy link
Contributor

@lyncasterc lyncasterc commented Feb 1, 2024

✨ Pull Request

📓 Referenced Issue

ℹ️ About the PR

It looks like there may have been some confusion in the original issue as to exactly what the functional requirements should be of this new feature. Based on your comments, my interpretation was that the user should be able to pinch-to-zoom on a trackpad in the preview area, which should zoom the devices in or out by updating the zoomFactor state variable.

  • Initially, I only added a setZoomFactor reducer and added an onWheel event listener in the Previewer component. This only worked when the mouse was not hovering over a webview element. When it was, the event listener would not fire. I did some searching through the docs and apparently "You can not add keyboard, mouse, and scroll event listeners to webview."
  • GIven that limitation, I decided that this would probably require IPC. So I used ipcRenderer and ipcMain modules to communicate the zoom gesture from the WebView to the main process.
  • Because pinch-to-zoom essentially allows "infinite" zoom steps, I had to rewrite the zoomIn and zoomOut to cover the instances when the user might pinch-to-zoom and fall in-between one of the predefined zoom steps, and then later use the zoom buttons. I rewrote the reducers to find the nearest predefined step (i.e. if the user pinched-to-zoom and landed on 40%, and then later used the zoom in button, it should go up to 50%. Or if they zoomed out from 40%, it should go to 33%).
  • I also made it so that the renderer.individualZoomFactor and renderer.zoomFactor in the Electron store are based on actual values instead of indices of the zoomSteps array. Again, because pinch-to-zoom allows "infinite" steps, I wanted to make it so that the application would remember the last zoomFactor even if it fell in between two pre-defined zoom steps.

I do have some concerns about the performance of this feature, as pinching-to-zoom can cause hundreds of re-rerenders, which sometimes caused some visible stuttering. The only other solution I could think of was updating the scale of the webview tags directly through CSS using a ref, and using some kind of debouncing function to only update the zoomFactor once the user has stopped pinching. I tested this and while it was visibly smoother, I ultimately decided against it because it required a lot more work-arounds and seemed a bit like an anti-pattern for React overall.

Hopefully I haven't missed anything. I'd be happy to make any requested changes to this implementation!

(sidenote: are the devices supposed to have that white space underneath? The prod application doesn't seem to have this but running the app locally, even on the master branch does for me.)

🖼️ Testing Scenarios / Screenshots

My.Movie.5.mp4

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.

Zoom in/out of devices on trackpad pinch gesture in the preview area
1 participant