Merge pull request #2
Conversation
Fixes NEMO#107
|
Looks good to me. I tested this on N950 and works fine. |
| function doPinchZoom(zoom,center,centerPrev) | ||
| { | ||
| var sc=zoom*contentsScale | ||
| if(sc>=0.5 && sc<=10 ){ |
There was a problem hiding this comment.
nitpick: Please use consistent spacing
There was a problem hiding this comment.
Does it really make sense to zoom out to 0.5? And, why 0.5?
There was a problem hiding this comment.
My spacing in the IDE is wrong, I will fix it.
About Zoom limitations: I think that some times it is usefull to zoom out little bit to see more on the screen, but more than 0.5 would be too small. Also double tap on the screen zooms first in and then out to 0.5. Do you think it shouldn't be still 1?
In some other browsers, there is pinch zooming out, but after releasing pinch it will zoom back to screen width. That would be quite easy to do.
And what about maximum zoom, I set it 10, should there be limitation?
There was a problem hiding this comment.
I can see how zoom <1 might feel nicer. I think I would like if it bounced back to 1 when released, personally.
You decide on the maximum. I have no strong opinion.
|
Looks OK other than the above. I haven't tested. |
|
I made the changes, now it bounces back to screen width, if zoom out is less than screen width. |
| @@ -56,9 +56,10 @@ Item { | |||
| width: parent.width | |||
| height: parent.height | |||
|
|
|||
| x: webView.contentX < 0 ? -webView.contentX : webView.contentX > webView.contentWidth-webView.width | |||
| //in case zoom is active don't move the header along panning | |||
| x: webView.zoomActive?0 : webView.contentX < 0 ? -webView.contentX : webView.contentX > webView.contentWidth-webView.width | |||
There was a problem hiding this comment.
Please use parenthesis with ternary if, especially if you have two of them.
|
Code seems good, although formatting (especially spacing) is very inconsistent, and there are lines of commented out code that you could remove. I'll give it a try, and we can merge if it goes well. Thanks for the contribution, helium is a bit neglected :) |
|
Works well. Will merge, tag, and release. Thanks. |
Fixes NEMO#107: Pinch zoom support
Fixes NEMO#107: Pinch zoom support.