Merge lp:~verzegnassi-stefano/ubuntu-docviewer-app/uitk13-lok-page into lp:ubuntu-docviewer-app

[Stefano Verzegnassi]

* WORKAROUND: make the lok-viewer header static (avoid unpredictable binding)
* Use new PageHeader and ScrollView components
* UI: Show an empty header when loading LibreOffice

* WORKAROUND: make the lok-viewer header static (avoid unpredictable binding)
* Use new PageHeader and ScrollView components
* UI: Show an empty header when loading LibreOffice

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

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

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

review: Approve (continuous-integration)

[Roman Shchekin (mrqtros)]

See inline comments!

review: Needs Information

[Stefano Verzegnassi (verzegnassi-stefano)]

> Haha, are you enjoyed deleting this code? ;)
Ahah, you can be sure of it! :)

> Why did you switch it [PageHeader.flickable] off? Just for information.
I was having trouble in setting the anchors for the content of the ViewerPage[1], and the LibreOffice Viewer had strange bindings that were causing a continuous flickering and reloading of the document content, every time the header was changing its status (i.e. visible/hidden).

I spent some time on it again (after you review), and now it seems to behave correctly. Pushing the new commit...

[1] We load LOViewPage asynchronously, but the loading logic is placed in a different file.

[lp:~verzegnassi-stefano/ubuntu-docviewer-app/uitk13-lok-page]

321. By Stefano Verzegnassi on 2016-03-30

Fixed flickable

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

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

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

review: Approve (continuous-integration)

=== modified file 'src/app/qml/common/ViewerPage.qml'
--- src/app/qml/common/ViewerPage.qml	2015-11-30 12:12:10 +0000
+++ src/app/qml/common/ViewerPage.qml	2016-03-30 11:10:56 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Stefano Verzegnassi <[e-mail]>
+ * Copyright (C) 2015, 2016 Stefano Verzegnassi <[e-mail]>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -35,7 +35,21 @@
 
     Loader {
         id: contentLoader
-        anchors.fill: parent
+        anchors {
+            left: parent.left
+            right: parent.right
+            bottom: parent.bottom
+
+            top: {
+                if (!viewerPage.header)
+                    return parent.top
+
+                if (!viewerPage.header.flickable)
+                    return viewerPage.header.bottom
+                else
+                    return parent.top
+            }
+        }
 
         asynchronous: true
         sourceComponent: viewerPage.contents
@@ -45,7 +59,22 @@
 
     Item {
         id: splashScreenItem
-        anchors.fill: parent
+
+        anchors {
+            left: parent.left
+            right: parent.right
+            bottom: parent.bottom
+
+            top: {
+                if (!viewerPage.header)
+                    return parent.top
+
+                if (!viewerPage.header.flickable)
+                    return viewerPage.header.bottom
+                else
+                    return parent.top
+            }
+        }
 
         visible: contentLoader.status != Loader.Ready
         enabled: visible

=== modified file 'src/app/qml/loView/KeybHelper.js'
--- src/app/qml/loView/KeybHelper.js	2015-12-14 00:40:55 +0000
+++ src/app/qml/loView/KeybHelper.js	2016-03-30 11:10:56 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Stefano Verzegnassi
+ * Copyright (C) 2015, 2016 Stefano Verzegnassi
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -14,83 +14,59 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
  
+// Here we handle all the key events that are not
+// recognised by UITK ScrollView
+
 function parseEvent(event) {
-    var pixelDiff = 5;
-
     var view = loPage.contentItem.loView
     var isPresentation = view.document.documentType === LibreOffice.Document.PresentationDocument
 
     if (event.key == Qt.Key_PageUp) {
-        if (isPresentation)
+        if (isPresentation) {
             view.currentPart -= 1
-        else
-            view.moveView("vertical", -view.height)
- 
+            event.accepted = true
+        }
         return;
     }
  
     if (event.key == Qt.Key_PageDown) {
-        if (isPresentation)
+        if (isPresentation) {
             view.currentPart += 1
-        else
-            view.moveView("vertical", view.height)
- 
+            event.accepted = true
+        }
         return;
     }
  
     if (event.key == Qt.Key_Home) {
-        if (event.modifiers & Qt.ControlModifier) {
-            view.contentX = 0
-            view.contentY = 0
+        if (event.modifiers & Qt.ControlModifier)
             view.currentPart = 0
-        } else {
-            view.contentX = 0
-            view.contentY = 0
-        }
+
+        event.accepted = false
+        return
     }
 
     if (event.key == Qt.Key_End) {
-        if (event.modifiers & Qt.ControlModifier) {
-            view.contentX = view.contentWidth - view.width
-            view.contentY = view.contentHeight - view.height
-            console.log(view.currentPart, view.document.partsCount - 1)
+        if (event.modifiers & Qt.ControlModifier)
             view.currentPart = view.document.partsCount - 1
-        } else {
-            view.contentX = view.contentWidth - view.width
-            view.contentY = view.contentHeight - view.height
-        }
-    }
 
-    if (event.key == Qt.Key_Up) {
-        view.moveView("vertical", -pixelDiff)
-        return;
-    }
- 
-    if (event.key == Qt.Key_Down) {
-        view.moveView("vertical", pixelDiff)
-        return;
-    }
- 
-    if (event.key == Qt.Key_Left) {
-        view.moveView("horizontal", -pixelDiff)
-        return;
-    }
- 
-    if (event.key == Qt.Key_Right) {
-        view.moveView("horizontal", pixelDiff)
-        return;
+        event.accepted = false
+        return
     }
 
     if (event.key == Qt.Key_Plus) {
         if (event.modifiers & Qt.ControlModifier) {
-            view.zoomFactor = Math.max(4.0, view.zoomFactor + 0.25)
+            view.setZoom(Math.min(view.zoomSettings.maximumZoom, view.zoomSettings.zoomFactor + 0.25))
         }
+
+        return
     }
 
     if (event.key == Qt.Key_Minus) {
         if (event.modifiers & Qt.ControlModifier) {
-            view.zoomFactor = Math.min(0.5, view.zoomFactor - 0.25)
+            view.setZoom(Math.max(view.zoomSettings.minimumZoom, view.zoomSettings.zoomFactor - 0.25))
         }
+
+        return
     }
 
 

=== modified file 'src/app/qml/loView/LOViewDefaultHeader.qml'
--- src/app/qml/loView/LOViewDefaultHeader.qml	2015-11-30 12:12:10 +0000
+++ src/app/qml/loView/LOViewDefaultHeader.qml	2016-03-30 11:10:56 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2015 Canonical, Ltd.
+ * Copyright (C) 2014-2016 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -16,68 +16,55 @@
 
 import QtQuick 2.4
 import Ubuntu.Components 1.3
-import QtQuick.Layouts 1.1
 import Ubuntu.Components.Popups 1.3
 import DocumentViewer.LibreOffice 1.0 as LibreOffice
 import DocumentViewer 1.0
 
-PageHeadState {
-    id: rootItem
-
-    property Page targetPage
-    head: targetPage.head
-
-    contents: RowLayout {
-        anchors.fill: parent
-        anchors.rightMargin: units.gu(2)
-        spacing: units.gu(1)
-
-        Column {
-            id: layout
-            Layout.fillWidth: true
-
-            Label {
-                anchors { left: parent.left; right: parent.right }
-                elide: Text.ElideMiddle
-                font.weight: Font.DemiBold
-                text: targetPage.title
-            }
-            Label {
-                anchors { left: parent.left; right: parent.right }
-                elide: Text.ElideMiddle
-                textSize: Label.Small
-                text: {
-                    if (!targetPage.contentItem)
-                        return i18n.tr("Loading...")
-
-                    switch(targetPage.contentItem.loDocument.documentType) {
-                    case 0:
-                        return i18n.tr("LibreOffice text document")
-                    case 1:
-                        return i18n.tr("LibreOffice spread sheet")
-                    case 2:
-                        return i18n.tr("LibreOffice presentation")
-                    case 3:
-                        return i18n.tr("LibreOffice Draw document")
-                    case 4:
-                        return i18n.tr("Unknown LibreOffice document")
-                    default:
-                        return i18n.tr("Unknown type document")
-                    }
+PageHeader {
+    id: defaultHeader
+
+    property var targetPage
+
+    contents: ListItemLayout {
+        anchors.centerIn: parent
+
+        title {
+            elide: Text.ElideMiddle
+            font.weight: Font.DemiBold
+            text: defaultHeader.title
+        }
+
+        subtitle {
+            textSize: Label.Small
+            text: {
+                if (!targetPage.contentItem)
+                    return i18n.tr("Loading...")
+
+                switch(targetPage.contentItem.loDocument.documentType) {
+                case LibreOffice.Document.TextDocument:
+                    return i18n.tr("LibreOffice text document")
+                case LibreOffice.Document.SpreadsheetDocument:
+                    return i18n.tr("LibreOffice spread sheet")
+                case LibreOffice.Document.PresentationDocument:
+                    return i18n.tr("LibreOffice presentation")
+                case LibreOffice.Document.DrawingDocument:
+                    return i18n.tr("LibreOffice Draw document")
+                case LibreOffice.Document.OtherDocument:
+                    return i18n.tr("Unknown LibreOffice document")
+                default:
+                    return i18n.tr("Unknown type document")
                 }
             }
         }
 
         ZoomSelector {
-            Layout.preferredWidth: units.gu(12)
-            Layout.preferredHeight: units.gu(4)
-
+            SlotsLayout.position: SlotsLayout.Trailing
             view: targetPage.contentItem.loView
             visible: targetPage.contentItem && (DocumentViewer.desktopMode || mainView.wideWindow)
         }
     }
 
-    actions: [
+    trailingActionBar.actions: [
         Action {
             // FIXME: Autopilot test broken... seems not to detect we're now using an ActionBar since the switch to UITK 1.3
             objectName: "gotopage"
@@ -86,12 +73,11 @@
             visible: targetPage.contentItem.loDocument.documentType == LibreOffice.Document.TextDocument
 
             onTriggered: {
-                PopupUtils.open(
-                            Qt.resolvedUrl("LOViewGotoDialog.qml"),
-                            targetPage,
-                            {
-                                view: targetPage.contentItem.loView
-                            })
+                var popupSettings = {
+                    view: targetPage.contentItem.loView
+                }
+
+                PopupUtils.open(Qt.resolvedUrl("LOViewGotoDialog.qml"), targetPage, popupSettings)
             }
         },
 

=== modified file 'src/app/qml/loView/LOViewPage.qml'
--- src/app/qml/loView/LOViewPage.qml	2016-02-03 21:35:53 +0000
+++ src/app/qml/loView/LOViewPage.qml	2016-03-30 11:10:56 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2015 Canonical, Ltd.
+ * Copyright (C) 2013-2016 Canonical, Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -32,9 +32,7 @@
     property bool isTextDocument: loPage.contentItem && (loPage.contentItem.loDocument.documentType === LibreOffice.Document.TextDocument)
     property bool isSpreadsheet: loPage.contentItem && (loPage.contentItem.loDocument.documentType === LibreOffice.Document.SpreadsheetDocument)
 
-    title: DocumentViewer.getFileBaseNameFromPath(file.path);
-    flickable: isTextDocument ? loPage.contentItem.loView : null
-
+    header: defaultHeader
     splashScreen: Splashscreen { }
 
     content: FocusScope {
@@ -137,96 +135,102 @@
                     color: "#f5f5f5"
                 }
 
-                LibreOffice.Viewer {
-                    id: loView
-                    objectName: "loView"
+                ScrollView {
                     anchors.fill: parent
 
-                    documentPath: file.path
-
-                    function updateContentSize(tgtScale) {
-                        zoomSettings.zoomFactor = tgtScale
-                    }
-
-                    // 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 or protected by a password.")
-                            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);
-                        }
-                    }
-
-                    ScalingMouseArea {
-                        id: mouseArea
+                    // We need to set some custom event handler.
+                    // Forward the key events to the Viewer and
+                    // fallback to the ScrollView handlers if the
+                    // event hasn't been accepted.
+                    Keys.forwardTo: loView
+                    Keys.priority: Keys.AfterItem
+
+                    LibreOffice.Viewer {
+                        id: loView
+                        objectName: "loView"
                         anchors.fill: parent
-                        targetFlickable: loView
-                        onTotalScaleChanged: targetFlickable.updateContentSize(totalScale)
-
-                        thresholdZoom: minimumZoom + (maximumZoom - minimumZoom) * 0.75
-                        maximumZoom: {
-                            if (DocumentViewer.desktopMode || mainView.wideWindow)
-                                return 3.0
-
-                            return minimumZoom * 3
-                        }
-                        minimumZoom: {
-                            if (DocumentViewer.desktopMode || mainView.wideWindow)
-                                return loView.zoomSettings.minimumZoom
-
-                            switch(loView.document.documentType) {
-                            case LibreOffice.Document.TextDocument:
-                                return loView.zoomSettings.valueFitToWidthZoom
-                            case LibreOffice.Document.PresentationDocument:
-                                return loView.zoomSettings.valueAutomaticZoom
-                            default:
-                                return loView.zoomSettings.minimumZoom
-                            }
-                        }
-
-                        Binding {
-                            target: mouseArea
-                            property: "zoomValue"
-                            value: loView.zoomSettings.zoomFactor
-                        }
-                    }
-
-                    Scrollbar { flickableItem: loView; parent: loView.parent }
-                    Scrollbar { flickableItem: loView; parent: loView.parent; align: Qt.AlignBottom }
-
-                    Label {
-                        anchors.centerIn: parent
-                        parent: loPage
-                        textSize: Label.Large
-                        text: i18n.tr("This sheet has no content.")
-                        visible: loPage.isSpreadsheet && loView.contentWidth <= 0 && loView.contentHeight <= 0
+
+                        documentPath: file.path
+
+                        Keys.onPressed: KeybHelper.parseEvent(event)
+
+                        function updateContentSize(tgtScale) {
+                            zoomSettings.zoomFactor = tgtScale
+                        }
+
+                        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 or protected by a password.")
+                                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);
+                            }
+                        }
+
+                        ScalingMouseArea {
+                            id: mouseArea
+                            anchors.fill: parent
+                            targetFlickable: loView
+                            onTotalScaleChanged: targetFlickable.updateContentSize(totalScale)
+
+                            thresholdZoom: minimumZoom + (maximumZoom - minimumZoom) * 0.75
+                            maximumZoom: {
+                                if (DocumentViewer.desktopMode || mainView.wideWindow)
+                                    return 3.0
+
+                                return minimumZoom * 3
+                            }
+                            minimumZoom: {
+                                if (DocumentViewer.desktopMode || mainView.wideWindow)
+                                    return loView.zoomSettings.minimumZoom
+
+                                switch(loView.document.documentType) {
+                                case LibreOffice.Document.TextDocument:
+                                    return loView.zoomSettings.valueFitToWidthZoom
+                                case LibreOffice.Document.PresentationDocument:
+                                    return loView.zoomSettings.valueAutomaticZoom
+                                default:
+                                    return loView.zoomSettings.minimumZoom
+                                }
+                            }
+
+                            Binding {
+                                target: mouseArea
+                                property: "zoomValue"
+                                value: loView.zoomSettings.zoomFactor
+                            }
+                        }
+
+                        Label {
+                            anchors.centerIn: parent
+                            parent: loPage
+                            textSize: Label.Large
+                            text: i18n.tr("This sheet has no content.")
+                            visible: loPage.isSpreadsheet && loView.contentWidth <= 0 && loView.contentHeight <= 0
+                        }
                     }
                 }
             }
@@ -265,12 +269,20 @@
         }
     }
 
-    // *** HEADER ***
-    state: "default"
-    states: [
-        LOViewDefaultHeader {
-            name: "default"
-            targetPage: loPage
-        }
-    ]
+
+    /*** Headers ***/
+
+    LOViewDefaultHeader {
+        id: defaultHeader
+        visible: loPage.loaded
+        title: DocumentViewer.getFileBaseNameFromPath(file.path);
+        flickable: isTextDocument ? loPage.contentItem.loView : null
+        targetPage: loPage
+    }
+
+    PageHeader {
+        id: loadingHeader
+        visible: !loPage.loaded
+        // When we're still loading LibreOffice, show an empty header
+    }
 }