ubuntu-touch-coreapps-reviewers team mailing list archive

[Stefano Verzegnassi <stefano92.100@xxxxxxxxx>]

Stefano Verzegnassi has proposed merging lp:~verzegnassi-stefano/ubuntu-docviewer-app/pdf-toc-improvements into lp:ubuntu-docviewer-app.

Commit message:
- Fix #1432696: Show a downward chevron as 'back' action in ToC page
- Fix #1435304: Highlight the current page in the table of contents
- Fix: Make page header visible when the ToC view is expanded

Requested reviews:
  Ubuntu Document Viewer Developers (ubuntu-docviewer-dev)
Related bugs:
  Bug #1432696 in Ubuntu Document Viewer App: "[SDK] Bottom edge header improvement"
  https://bugs.launchpad.net/ubuntu-docviewer-app/+bug/1432696
  Bug #1435304 in Ubuntu Document Viewer App: "[pdfView] Highlight the ToC entry that includes the current visible page"
  https://bugs.launchpad.net/ubuntu-docviewer-app/+bug/1435304

For more details, see:
https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/pdf-toc-improvements/+merge/255518

- Fix #1432696: Show a downward chevron as 'back' action in ToC page
- Fix #1435304: Highlight the current page in the table of contents
- Fix: Make page header visible when the ToC view is expanded
-- 
Your team Ubuntu Document Viewer Developers is requested to review the proposed merge of lp:~verzegnassi-stefano/ubuntu-docviewer-app/pdf-toc-improvements into lp:ubuntu-docviewer-app.

=== modified file 'po/com.ubuntu.docviewer.pot'
--- po/com.ubuntu.docviewer.pot	2015-04-07 14:14:52 +0000
+++ po/com.ubuntu.docviewer.pot	2015-04-08 14:21:27 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-04-07 16:14+0200\n"
+"POT-Creation-Date: 2015-04-08 16:15+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@xxxxxx>\n"
@@ -189,7 +189,7 @@
 msgstr ""
 
 #: ../src/app/qml/documentPage/DocumentPage.qml:25
-#: /home/stefano/Progetti/doc-viewer/build-pdfview-show-hide-header-Desktop-Default/po/com.ubuntu.docviewer.desktop.in.in.h:1
+#: /home/stefano/Progetti/doc-viewer/build-ubuntu-docviewer-app-Desktop-Default/po/com.ubuntu.docviewer.desktop.in.in.h:1
 msgid "Document Viewer"
 msgstr ""
 
@@ -216,11 +216,15 @@
 msgstr ""
 
 #. TRANSLATORS: "Contents" refers to the "Table of Contents" of a PDF document.
-#: ../src/app/qml/pdfView/PdfContentsPage.qml:25
+#: ../src/app/qml/pdfView/PdfContentsPage.qml:26
 #: ../src/app/qml/pdfView/PdfView.qml:37
 msgid "Contents"
 msgstr ""
 
+#: ../src/app/qml/pdfView/PdfContentsPage.qml:32
+msgid "Hide table of contents"
+msgstr ""
+
 #. TRANSLATORS: the first argument (%1) refers to the page currently shown on the screen,
 #. while the second one (%2) refers to the total pages count.
 #: ../src/app/qml/pdfView/PdfView.qml:34
@@ -264,6 +268,6 @@
 msgid "Loading..."
 msgstr ""
 
-#: /home/stefano/Progetti/doc-viewer/build-pdfview-show-hide-header-Desktop-Default/po/com.ubuntu.docviewer.desktop.in.in.h:2
+#: /home/stefano/Progetti/doc-viewer/build-ubuntu-docviewer-app-Desktop-Default/po/com.ubuntu.docviewer.desktop.in.in.h:2
 msgid "documents;viewer;pdf;reader;"
 msgstr ""

=== modified file 'src/app/qml/pdfView/PdfContentsPage.qml'
--- src/app/qml/pdfView/PdfContentsPage.qml	2015-03-12 19:00:33 +0000
+++ src/app/qml/pdfView/PdfContentsPage.qml	2015-04-08 14:21:27 +0000
@@ -18,43 +18,106 @@
 import QtQuick 2.0
 import Ubuntu.Components 1.1
 import QtQuick.Layouts 1.1
-import Ubuntu.Components.ListItems 1.0 as ListItem
+
+import "../upstreamComponents"
 
 Page {
     // TRANSLATORS: "Contents" refers to the "Table of Contents" of a PDF document.
     title: i18n.tr("Contents")
 
+    // Avoid a binding loop when using ListView.positionViewAtIndex()
+    flickable: null
+
+    head.backAction: Action {
+        text: i18n.tr("Hide table of contents")
+        iconName: "down"
+        onTriggered: pageStack.pop()
+    }
+
+    onActiveChanged: {
+        // If the header was hidden in the PdfPage, make it visible.
+        if (!pdfPage.header.visible)
+            pdfPage.header.visible = true;
+
+        // Find out the current page position in the ToC index
+        var i=0
+        while(!(pdfView.currentPageIndex >= poppler.tocModel.get(i).pageIndex &&
+              pdfView.currentPageIndex < poppler.tocModel.get(i+1).pageIndex)) {
+            i++
+        }
+
+        // Set highlighted index
+        view.currentIndex = i;
+
+        // Position view at the highlighted index
+        view.positionViewAtIndex(i, ListView.Center);
+    }
+
     ListView {
+        id: view
         anchors.fill: parent
+        clip: true
 
         model: poppler.tocModel
 
-        delegate: ListItem.Base {
-            showDivider: model.level == 0
-
-            onClicked: {
-                pdfView.positionAtIndex(model.pageIndex);
-                pageStack.pop();
+        delegate: ListItemWithActions {
+            id: delegate
+
+            width: parent.width
+            height: (model.level === 0) ? units.gu(7) : units.gu(6)
+
+            // Don't use 'selected' property here, since it shows a CheckBox
+            color: (view.currentIndex == model.index) ? Qt.lighter(UbuntuColors.lightGrey)
+                                                      : Theme.palette.normal.background
+
+            AbstractButton {
+                anchors.fill: parent
+
+                onClicked: {
+                    pdfView.positionAtIndex(model.pageIndex);
+                    pageStack.pop();
+                }
             }
 
             RowLayout {
-                anchors.fill: parent
-                anchors.leftMargin: units.gu(2) * model.level
+                anchors {
+                    fill: parent
+                    leftMargin: units.gu(1) + (units.gu(2) * model.level)
+                    rightMargin: units.gu(1)
+                }
 
                 spacing: units.gu(1)
 
                 Label {
                     Layout.fillWidth: true
 
-                    text: (typeof model.title === "undefined") ? "" : model.title;
+                    text: model.title
                     elide: Text.ElideRight
-                    Component.onCompleted: { if (model.level === 0); font.weight = Font.DemiBold; }
+
+                    font.weight: model.level == 0 ? Font.DemiBold : Font.Normal
+                    color: (model.level === 0) ? UbuntuColors.midAubergine
+                                               : Theme.palette.selected.backgroundText
                 }
 
                 Label {
-                    text: (typeof model.pageIndex === "undefined") ? "" : model.pageIndex + 1;
-                    Component.onCompleted: { if (model.level === 0); font.weight = Font.DemiBold; }
-                }
+                    text: model.pageIndex + 1
+                    font.weight: model.level == 0 ? Font.DemiBold : Font.Normal
+                    color: (model.level === 0) ? UbuntuColors.midAubergine
+                                               : Theme.palette.selected.backgroundText
+                }
+            }
+
+            Rectangle {
+                anchors {
+                    left: parent.left
+                    right: parent.right
+                    bottom: parent.bottom
+                }
+
+                height: units.gu(0.1)
+                visible: model.level == 0
+                color: (view.currentIndex === model.index) ? "transparent"
+                                                           : UbuntuColors.midAubergine
             }
         }
     }

=== modified file 'src/plugin/poppler-qml-plugin/pdftocmodel.cpp'
--- src/plugin/poppler-qml-plugin/pdftocmodel.cpp	2015-02-05 19:30:53 +0000
+++ src/plugin/poppler-qml-plugin/pdftocmodel.cpp	2015-04-08 14:21:27 +0000
@@ -72,6 +72,23 @@
     }
 }
 
+QVariantMap PdfTocModel::get(int index) const
+{
+    if (index < 0 && index > m_entries.length()) {
+        qWarning() << Q_FUNC_INFO << "Index not valid, return undefined";
+        return QVariantMap();
+    }
+
+    const TocEntry &item = m_entries.at(index);
+
+    QVariantMap map;
+    map["title"] = item.title;
+    map["pageIndex"] = item.pageIndex;
+    map["level"] = item.level;
+
+    return map;
+}
+
 void PdfTocModel::fillModel() {
     if (m_entries.count() != 0) {
         m_entries.clear();

=== modified file 'src/plugin/poppler-qml-plugin/pdftocmodel.h'
--- src/plugin/poppler-qml-plugin/pdftocmodel.h	2015-02-05 19:30:53 +0000
+++ src/plugin/poppler-qml-plugin/pdftocmodel.h	2015-04-08 14:21:27 +0000
@@ -54,6 +54,8 @@
     int rowCount(const QModelIndex & parent = QModelIndex()) const;
     QVariant data(const QModelIndex & index, int role = Qt::DisplayRole) const;
 
+    Q_INVOKABLE QVariantMap get(int index) const;
+
 Q_SIGNALS:
     void documentChanged();
     void countChanged();