-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Pinch zoom #1809
base: master
Are you sure you want to change the base?
Pinch zoom #1809
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
I'm not opposed to the idea of a pinch to zoom gesture. However, there are numerous issues here. First and foremost, noVNC already has a sort of pinch to zoom. Check core/rfb.js
and the _handleGesture()
method. It sends Ctrl+Scroll
to the remote, i.e server-side "zoom". I see that we need to find a method where the user gets to choose between the proposed client-side zoom and the existing server-side variant.
I also tested your code, and while pinch-to-zoom works fine, the biggest user-facing issue is that drag-and-drop is completely disabled now.
Another issue is that you seem to have disregarded the existing architecture and style of the code. In order for this to be maintainable, we need to keep things uniform and coherent. For example, core/input/gesturehandler.js
is only supposed to detect the actual gesture, not act upon it. That should be the responsibility of _handleGesture()
in RFB.
This comment was marked as off-topic.
This comment was marked as off-topic.
// // Agilicus Modified | ||
// document.getElementById("noVNC_view_drag_button") | ||
// .addEventListener('click', UI.toggleViewDrag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remove the code instead of uncommenting.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This work here seems to be in more of a proof of concept stage. So I'll go ahead and mark this as a draft. Feel free to switch it back once you feel it is in a state that is ready for review. |
Added auto panning and pinch zoom functionality.
My requirement is to make pinch zoom functionality to work on mobile devices and give seamless panning.
I used the pinch gesture inside gesturehandler.js and added an event handlers to make novnc canvas zoom in/zoom out.