Merge lp:~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-zoom-final into lp:ubuntu-docviewer-app

[Roman Shchekin]

Zoom in LO.

Zoom in LO.

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

FAILED: Continuous integration, rev:199
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/24/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/167/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/24/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-zoom-final]

200. By Roman Shchekin on 2015-11-21

Resolved conflict.

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:200
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/25/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/168/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/docviewer-app-ci/25/rebuild

review: Approve (continuous-integration)

[Stefano Verzegnassi (verzegnassi-stefano)]

I left 4 diff comments.
They are just a few small notes, your changes look good to me.

Tested on desktop, with both Ubuntu.Layouts modes. I didn't see any regression (cannot test touch events on my PC though).

Tested on the BQ, and works greatly as expected!

Nice work, Roman!

review: Approve

[Alan Pope 🍺🐧🐱 🦄 (popey)]

This is looking great.

One thing I noticed though is the zoom dropdown is always fixed on "Automatic (Fit width)" even if you choose another zoom level, or zoom in or out with the buttons. This means if you change the zoom level, you can never re-choose "Fit Width) because it thinks it's already ticked.

review: Needs Fixing

[Stefano Verzegnassi (verzegnassi-stefano)]

> One thing I noticed though is the zoom dropdown is always fixed on "Automatic
> (Fit width)" even if you choose another zoom level, or zoom in or out with the
> buttons. This means if you change the zoom level, you can never re-choose "Fit
> Width) because it thinks it's already ticked.

Honestly, I've missed that. Thanks!

Just had a look at it, it's not a bug introduced by this MP.

I've checked if the Viewer properly set the zoom mode to Manual when a "custom" (i.e. user-specified) value is set, and it does.

It's rather a bug in the ZoomSelector, from line 126:

selectedIndex: {
    if (loPageContentLoader.item.loView.zoomMode == LibreOffice.View.FitToWidth)
        return 0

    for (var i=0; i<textField.values.length; i++) {
        if (values[i] == loView.zoomFactor) {
            return i
        }
    }

    return -1
}

It returns:
    ReferenceError: loView is not defined

The 'for' loop in the code does not return any index, since the comparison between values[i] and loView.zoomFactor is never executed.
For that reason, the first entry in the ZoomSelector ("Automatic (Fit width)") is always selected.

I didn't take care of the current code of the ZoomSelector, since it was already planned to be replaced with some properly designed code.

This bug is already fixed in the UITK 1.3 branch, since it introduces the new ZoomSelector.

[Stefano Verzegnassi (verzegnassi-stefano)]

Anyway, and that's for sure, the work on zooming is not completed by this MP.

We still have three relevant things to do:

1) Add the double-tap-to-(un)zoom gesture.

2) Properly define the zoom behaviour (minimum zoom, maximum zoom, default value) for the following three form factors:
        - Touch device with screen.width < units.gu(80) - a.k.a. phones
        - Touch device with screen.width > units.gu(80) - a.k.a. tablet
        - Desktop
   We should also take care that the three types of documents we support through LibreOffice (Text, Spreadsheet and Presentation) have some different requirement.
   I started to have a look at this, but it needs to be planned accurately.

   In particular on phones we should limit the minimum zoom to the value set by the "Fit to width" zoom (e.g. Text document).

3) Introduce two new zoom modes: "Fit to Height" and "Automatic".
   "Automatic" mode should be a mix of "Fit to Height" and "Fit to Width". We need it in order to ensure that slides of presentation documents will be always fully visible, no matter which is the size/ratio of the application window.

[Alan Pope 🍺🐧🐱 🦄 (popey)]

Great! Looks good so far!

review: Approve

=== modified file 'po/com.ubuntu.docviewer.pot'
--- po/com.ubuntu.docviewer.pot	2015-10-31 20:37:19 +0000
+++ po/com.ubuntu.docviewer.pot	2015-11-21 19:45:28 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-10-31 23:16+0300\n"
+"POT-Creation-Date: 2015-11-21 22:13+0300\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -245,7 +245,7 @@
 
 #: ../src/app/qml/documentPage/DocumentPageSearchHeader.qml:27
 #: ../src/app/qml/loView/LOViewDefaultHeader.qml:70
-#: ../src/app/qml/loView/LOViewPage.qml:191
+#: ../src/app/qml/loView/LOViewPage.qml:208
 #: ../src/app/qml/pdfView/PdfViewDefaultHeader.qml:61
 #: ../src/app/qml/textView/TextViewDefaultHeader.qml:61
 msgid "Back"
@@ -378,8 +378,8 @@
 msgid "GO!"
 msgstr ""
 
-#: ../src/app/qml/loView/LOViewPage.qml:34
-#: ../src/app/qml/loView/LOViewPage.qml:189
+#: ../src/app/qml/loView/LOViewPage.qml:35
+#: ../src/app/qml/loView/LOViewPage.qml:206
 msgid "Slides"
 msgstr ""
 
@@ -458,10 +458,10 @@
 msgid "copy %1"
 msgstr ""
 
-#: /home/qtros/dev/ubuntu-docviewer-app-render-engine-tuning-build/po/com.ubuntu.docviewer.desktop.in.in.h:1
+#: /home/qtros/dev/build-ubuntu-docviewer-app-zoom-final-UbuntuSDK_for_armhf_GCC_ubuntu_sdk_15_04_vivid-Default/po/com.ubuntu.docviewer.desktop.in.in.h:1
 msgid "Document Viewer"
 msgstr ""
 
-#: /home/qtros/dev/ubuntu-docviewer-app-render-engine-tuning-build/po/com.ubuntu.docviewer.desktop.in.in.h:2
+#: /home/qtros/dev/build-ubuntu-docviewer-app-zoom-final-UbuntuSDK_for_armhf_GCC_ubuntu_sdk_15_04_vivid-Default/po/com.ubuntu.docviewer.desktop.in.in.h:2
 msgid "documents;viewer;pdf;reader;"
 msgstr ""

=== added file 'src/app/qml/common/ScalingPinchArea.qml'
--- src/app/qml/common/ScalingPinchArea.qml	1970-01-01 00:00:00 +0000
+++ src/app/qml/common/ScalingPinchArea.qml	2015-11-21 19:45:28 +0000
@@ -0,0 +1,55 @@
+import QtQuick 2.4
+
+PinchArea {
+    id: pinchArea
+
+    property var targetFlickable: null
+    property real totalScale: 1
+
+    onPinchUpdated: {
+        pinchUpdatedHandler(pinch)
+    }
+    onPinchFinished: {
+        pinchFinishedHandler()
+    }
+
+    // ------------------------ Desktop DEBUG
+
+//    MouseArea {
+//        id: testMa
+//        anchors.fill: parent
+//        visible: Qt.platform.os == "linux"
+//        z: 19
+//        acceptedButtons: Qt.RightButton
+
+//        property int touchPointY
+//        property int touchPointX
+//        onPressed: {
+//            touchPointY = mouse.y
+//            touchPointX = mouse.x
+//        }
+//        onPositionChanged: {
+//            var sc = (1 + (mouse.y - touchPointY) / 200)
+//            pinchUpdatedHandler({"center" : { "x" : mouse.x, "y" : mouse.y }, "scale" : sc })
+//        }
+//        onReleased: {
+//            pinchFinishedHandler()
+//        }
+//    }
+
+    // ------------------------ Desktop DEBUG end
+
+    function pinchUpdatedHandler(pinch) {
+        targetFlickable.scale = pinch.scale
+    }
+
+    function pinchFinishedHandler() {
+        var pt = pinchArea.mapFromItem(targetFlickable, -targetFlickable.contentX , -targetFlickable.contentY )
+        // console.log("pinchFinishedHandler", -myItem.contentX, -myItem.contentY, Math.round(pt.x), Math.round(pt.y))
+        targetFlickable.contentX = -pt.x
+        targetFlickable.contentY = -pt.y
+
+        totalScale = targetFlickable.scale * totalScale
+        targetFlickable.scale = 1
+    }
+}

=== modified file 'src/app/qml/loView/LOViewPage.qml'
--- src/app/qml/loView/LOViewPage.qml	2015-11-13 17:31:09 +0000
+++ src/app/qml/loView/LOViewPage.qml	2015-11-21 19:45:28 +0000
@@ -22,6 +22,7 @@
 import "../upstreamComponents"
 
 import "../common/utils.js" as Utils
+import "../common"
 import "KeybHelper.js" as KeybHelper
 
 PageWithBottomEdge {
@@ -113,7 +114,7 @@
                                 }
 
                                 ItemLayout {
-                                    item: "loView"
+                                    item: "pinchArea"
                                     anchors.fill: parent
 
                                     // Keyboard events
@@ -139,58 +140,74 @@
                     }
                 ]
 
-                LibreOffice.Viewer {
-                    id: loView
-                    objectName: "loView"
-                    Layouts.item: "loView"
+                ScalingPinchArea {
+                    id: pinchArea
+                    objectName: "pinchArea"
+                    Layouts.item: "pinchArea"
+
+                    targetFlickable: loView
+                    onTotalScaleChanged: targetFlickable.updateContentSize(totalScale)
+
                     anchors {
-                        left: parent.left
-                        right: parent.right
                         top: parent.top
+                        left: parent.left
+                        right: parent.right
                         bottom: bottomBar.top
                     }
 
-                    clip: true
-                    documentPath: file.path
-
-                    // Keyboard events
-                    focus: true
-                    Keys.onPressed: KeybHelper.parseEvent(event)
-
-                    Component.onCompleted: {
-                        // WORKAROUND: Fix for wrong grid unit size
-                        flickDeceleration = 1500 * units.gridUnit / 8
-                        maximumFlickVelocity = 2500 * units.gridUnit / 8
-                        loPageContent.forceActiveFocus()
-                    }
-
-                    onErrorChanged: {
-                        var errorString;
-
-                        switch(error) {
-                        case LibreOffice.Error.LibreOfficeNotFound:
-                            errorString = i18n.tr("LibreOffice binaries not found.")
-                            break;
-                        case LibreOffice.Error.LibreOfficeNotInitialized:
-                            errorString = i18n.tr("Error while loading LibreOffice.")
-                            break;
-                        case LibreOffice.Error.DocumentNotLoaded:
-                            errorString = i18n.tr("Document not loaded.\nThe requested document may be corrupt.")
-                            break;
-                        }
-
-                        if (errorString) {
-                            loPage.pageStack.pop()
-
-                            // We create the dialog in the MainView, so that it isn't
-                            // initialized by 'loPage' and keep on working after the
-                            // page is destroyed.
-                            mainView.showErrorDialog(errorString);
-                        }
-                    }
-
-                    Scrollbar { flickableItem: loView; parent: loView.parent }
-                    Scrollbar { flickableItem: loView; parent: loView.parent; align: Qt.AlignBottom }
+                    LibreOffice.Viewer {
+                        id: loView
+                        objectName: "loView"
+                        Layouts.item: "loView"
+
+                        anchors.fill: parent
+
+                        clip: true
+                        documentPath: file.path
+
+                        // Keyboard events
+                        focus: true
+                        Keys.onPressed: KeybHelper.parseEvent(event)
+
+                        Component.onCompleted: {
+                            // WORKAROUND: Fix for wrong grid unit size
+                            flickDeceleration = 1500 * units.gridUnit / 8
+                            maximumFlickVelocity = 2500 * units.gridUnit / 8
+                            loPageContent.forceActiveFocus()
+                        }
+
+                        function updateContentSize(tgtScale) {
+                            zoomFactor = tgtScale
+                        }
+
+                        onErrorChanged: {
+                            var errorString;
+
+                            switch(error) {
+                            case LibreOffice.Error.LibreOfficeNotFound:
+                                errorString = i18n.tr("LibreOffice binaries not found.")
+                                break;
+                            case LibreOffice.Error.LibreOfficeNotInitialized:
+                                errorString = i18n.tr("Error while loading LibreOffice.")
+                                break;
+                            case LibreOffice.Error.DocumentNotLoaded:
+                                errorString = i18n.tr("Document not loaded.\nThe requested document may be corrupt.")
+                                break;
+                            }
+
+                            if (errorString) {
+                                loPage.pageStack.pop()
+
+                                // We create the dialog in the MainView, so that it isn't
+                                // initialized by 'loPage' and keep on working after the
+                                // page is destroyed.
+                                mainView.showErrorDialog(errorString);
+                            }
+                        }
+
+                        Scrollbar { flickableItem: loView; parent: loView.parent }
+                        Scrollbar { flickableItem: loView; parent: loView.parent; align: Qt.AlignBottom }
+                    }
                 }
 
                 // TODO: When we'll have to merge this with the zooming branch, replace this