Bug 106453

Hi,

I would like to work on this issue.

(Please correct me if i am thinking in the wrong direction)

To create this feature, maybe we could add an eventlistener with the scroll event that connects MouseEvent.ctrlKey and WheelEvent.deltaY with the already implemented zoom logic.

Pranav told me a few days ago that this feature was part of the original Leaflet, but without requiring the Ctrl key being pressed, and hence was turned off. So finding the original code, and reactivating it with the addition of requiring the Ctrl key could be a possible approach here.

Do a git blame in Map.Scroll.js, and see Mihai Varga's earlier commits to find which was the one that disabled this function. That should help you get started. 

This task will be a bit more challenging, good luck! :)

Hi

I went through the history of 'Map.Scroll.js' and found the following commit [1] relevant, i believe this(and a couple other commits [2], [3] done on the same day) are the commits you were were referring to.

So, I took the appropriate code snippet from 'Map.ScrollWheelZoom.js'( which was renamed to 'Map.Scroll.js' in [3]) and used it to implement this feature by using the ctrlKey property of any mouse event.
Though there are some minor issues, but it works fine for the most part.

I have created a patch here:
https://gerrit.libreoffice.org/#/c/35288/


[1] https://gerrit.libreoffice.org/gitweb?p=online.git;a=commit;h=e7fc840c9e89eb8e7c6430183c1302bf6682417e
[2] https://gerrit.libreoffice.org/gitweb?p=online.git;a=commit;h=bfa5eda5036062eecf4191bf79dd3d75dfefab34
[3] https://gerrit.libreoffice.org/gitweb?p=online.git;a=commit;h=d33f71d32d1cef48fdee63ee0d6c5c1f4d9d048c

Awesome work. Just tried it, loved the feature.

(In reply to Aditya Dewan from comment #4)
> Hi
> 
> I went through the history of 'Map.Scroll.js' and found the following commit
> [1] relevant, i believe this(and a couple other commits [2], [3] done on the
> same day) are the commits you were were referring to.
> 
> So, I took the appropriate code snippet from 'Map.ScrollWheelZoom.js'( which
> was renamed to 'Map.Scroll.js' in [3]) and used it to implement this feature
> by using the ctrlKey property of any mouse event.
> Though there are some minor issues, but it works fine for the most part.

Can you describe these minor issues you noticed ?

> 
> I have created a patch here:
> https://gerrit.libreoffice.org/#/c/35288/
> 

All looks good, except nitpicks, please check the comments there.

Thanks :)

*According to my usage on chrome browser(only)
Most of the issues that i noticed seem to be due to conflicts between the normal zoom feature of a webpage i.e (ctrl + scroll) and the feature that we have implemented. Let me explain some scenerios.

1. Keep your mouse outside the doc(somewhere between the doc and scrollbar) and try to zoom by ctrl + scroll. Sometimes the document gets zoomed and sometimes the whole webpage gets zoomed.

2. Lets say you have zoomed in a document to 150%, and now you zoom in/out the webpage. the document zoom gets reset to 100%.(this isn't specifically related to this feature, but is general)

Closing it now. Fixed in - https://gerrit.libreoffice.org/#/c/35288/

Next time, please also use bug number as prefix to the commit message. That way gerrit automtically notifies the bugzilla that fix has been pushed etc. Eg: see https://gerrit.libreoffice.org/gitweb?p=online.git;a=commit;h=bf8f8f17384e0b31345eac20493d4b2acf670bd9 as an example.

OK, will do that from now onwards.

So - a nice feature, but also something of a problem.

LibreOffice has to re-render (to pixels) the same scene <N> times where <N> is the number of clients with different zoom levels. User's experience of latency is extremely sensitive to the time it takes to render a tile, we already have a highish network latency and I'm a little concerned here.

So - essentially making it easy to choose different zoom levels is likely to make LibreOffice Online some multiple of times slower when many people are using this; so - this is really not the ideal easy-hack =)

To mitigate the potential problem I would like to work to reduce the number of zoom levels possible - particularly subtly different / fractional zoom levels are a nightmare. If the zoom just flicks between say 4-5 pre-set zoom levels - then this makes much more sense: we really want to make sure that our say 50 clients are not all at a different zoom.
    + it would be good to investigate what makes for a good range of levels there - the smaller the better to be usable and then have an array of these that we switch between. Of course touch devices can pinch-to-zoom but really we need to reduce the set of acceptable levels there too I think ;-)

Thanks !