viewer
viewer copied to clipboard
Pinch to zoom
Here is my initial pinch-to-zoom implementation. This should resolve #916 and #917.
I have not committed the compiled code yet, to make rebasing easier.
Some explanation of the changes:
In https://github.com/nextcloud/viewer/pull/1952/commits/003f31fe70550da3c8046e80124784a425335b4c I have switched from mouse events to more generic pointer events (see https://developer.mozilla.org/en-US/docs/Web/API/Pointer_events).
https://github.com/nextcloud/viewer/pull/1952/commits/e96662cc299e6d665b7b7d7112ee98fbe744b649 is the actual implementation.
- I have re-used the original zoom handling code - a significant part of the old
updateZoommethod was extracted toupdateZoomAndShiftand I created an additional common methodupdateShift. - Co-ordinates of the two touches are kept in
pointerCache. - Pinch-zoom calculations are explained in the comments.
I committed https://github.com/nextcloud/viewer/pull/1952/commits/aeab2bd8f159fae5553150b293d2c12fa7f353ca separately, to make it easier to follow the changes.
https://github.com/nextcloud/viewer/pull/1952/commits/1f0fcedc9dfa1ecc86a3ab59418c1e151425a1b7 was required after recent changes in master - as I removed the width/height: 100% from CSS.
I have the following questions:
- Regarding https://github.com/nextcloud/viewer/pull/1952/commits/1f0fcedc9dfa1ecc86a3ab59418c1e151425a1b7 - an alternative solution would be to bring back
width/height: 100%. I am not sure which one is better. Any suggestions? - The
pointerMoveandpointerUphandlers are now attached all the time. I am not sure what should be done here: https://github.com/nextcloud/viewer/blob/698afa42df0c8976e797a68dec8c8ee8f2dc9c89/src/components/Images.vue#L219 and here: https://github.com/nextcloud/viewer/blob/698afa42df0c8976e797a68dec8c8ee8f2dc9c89/src/components/Images.vue#L223 Probably also a permanent handler forpointerOutthat would invokeresetZoom? I do not know how to test this case. Suggestions welcome. - I have tested on Android and iPad. Would be good to verify on an iPhone.
- I have noticed that recent changes to
NcModal(https://github.com/nextcloud-libraries/nextcloud-vue/pull/4350) affect zooming behaviour. Now, when the image is enlarged a bit from 100%, a scrollbar is shown, and this causes a slight shift in image position. This happens on Chrome and Chrome Mobile (for some reason Firefox does not show the scrollbar). This is not a major problem, but is ~~quite~~ a bit annoying. I wonder if we should try to have some workaround in this PR or leave it for a separate PR.
Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.
We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process.
Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6
Thank you for contributing to Nextcloud and we hope to hear from you soon!
@starypatyk I don't have much time for Viewer lately, thank you for this PR (and your patience :see_no_evil:)
@skjnldsv No worries. :wink: Thanks for letting me know. Waiting patiently then. :snail:
@starypatyk I don't have much time for Viewer lately, thank you for this PR (and your patience 🙈)
Hi @skjnldsv,
Any chance to go forward with this PR?
FYI - I have rebased and re-checked the implementation. Seems to behave decently. Tested as previously on Android and iPad.
This still requires some clean-up - see my questions in the first comment (I have updated commit references after rebase in the original comment).
/compile /
Patience came to an end, let get this in! Sorry again @starypatyk!
Btw, if you're interested, I sketched an RFC for an improved Viewer version 3.0. If you have some commens or feedback, feel free to check #2395 :tada:
cypress fix: https://github.com/nextcloud/viewer/pull/2436
Fixing the public view rendering :)
@skjnldsv - Great to see this merged finally! :wink: Thanks :+1: