ubuntu-touch-coreapps-reviewers team mailing list archive
-
ubuntu-touch-coreapps-reviewers team
-
Mailing list archive
-
Message #06146
Re: [Merge] lp:~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-zoom-final into lp:ubuntu-docviewer-app
Review: Approve
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!
Diff comments:
> === modified file 'po/com.ubuntu.docviewer.pot'
Not important this time: as long as we don't disable auto-update of the .pot during the building, would you mind to "bzr revert po/" before commit a new revision?
An update of the translation template may easily cause text conflict in other branches.
As a rule, is it okay for you to update it only when we import new strings for translating?
>
> === 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 {
This may be a bit unconfortable to uncomment if we'll need to do some debug. :)
Not a real problem though, but if you could use /* these */, that would become a bit simple.
> +// id: testMa
<pedantic_mode> No other object will access to 'testMa' MouseArea properties, so we can remove the setter for 'id' property </pedantic_mode>
> +// 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
> @@ -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"
You can remove
Layouts.item: "loView"
Since it's not longer used by Ubuntu.Layouts
> +
> + 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
--
https://code.launchpad.net/~mrqtros/ubuntu-docviewer-app/ubuntu-docviewer-app-zoom-final/+merge/278248
Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app.
