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

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

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

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!

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.

> 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.

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.

Great! Looks good so far!