[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
The proposal to merge lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app has been updated. Status: Approved => Rejected For more details, see: https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 -- Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Proposed for merging a new branch, depending on trunk and including the changes made by Michael Zanetti. For this reason, I mark this MP as rejected. Ref. https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/fix-layout-with-icons-2/+merge/263369 -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
This branch has been proposed against a development branch that still requires some work. Probably there are other QtQuick.Layouts in the app that may need the fix proposed here. During the weekend I'll open a branch that refers to lp:ubuntu-docviewer-app and will include this MP. -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Did this branch ever land? It's marked as Approved instead of Merged, so it appears in the list of branches to review. If it landed, could it be marked as Merged, or alternatively as Rejected? Thanks! -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
The proposal to merge lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 -- Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Review: Approve I had that doubt because the patch didn't seem to be related to the prerequisite branch. In this case it's OK for me. Thank you again for giving a look at the branch. :D -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Hi Stefano. No, I was asked by Alan to have a look at your branch and had to do those changes to make it work with Qt 5.5 and my High-resolution screen, otherwise it would end up in a loop requesting those images over and over again at startup. While the root cause for that loop is a bug in Qt, fixing the layout usage is a good idea anyways. As I did those changes already I just pushed the branch so the fix isn't lost. -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Review: Needs Information Hi Michael, Thank you for the patch! I wonder if you intentionally chose lp:~verzegnassi-stefano/ubuntu-docviewer-app/attempt-fix-zooming-1 as prerequisite branch. Also, it seems there's a conflict with the translation template. -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:99 http://91.189.93.70:8080/job/ubuntu-docviewer-app-ci/184/ Executed test runs: FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2424/console FAILURE: http://91.189.93.70:8080/job/ubuntu-docviewer-app-utopic-amd64-ci/81/console FAILURE: http://91.189.93.70:8080/job/ubuntu-docviewer-app-vivid-amd64-ci/87/console Click here to trigger a rebuild: http://91.189.93.70:8080/job/ubuntu-docviewer-app-ci/184/rebuild -- https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072 Your team Ubuntu Document Viewer Developers is requested to review the proposed merge of lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app. -- Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers More help : https://help.launchpad.net/ListHelp
[Ubuntu-touch-coreapps-reviewers] [Merge] lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into lp:ubuntu-docviewer-app
Michael Zanetti has proposed merging
lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into
lp:ubuntu-docviewer-app with
lp:~verzegnassi-stefano/ubuntu-docviewer-app/attempt-fix-zooming-1 as a
prerequisite.
Commit message:
properly set icon sizes inside layouts
Requested reviews:
Ubuntu Document Viewer Developers (ubuntu-docviewer-dev)
For more details, see:
https://code.launchpad.net/~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons/+merge/254072
QtQuick.Layouts ignore width/height and only act on
implicitWidth/implicitHeight. Given that the implicit size of an Icon does not
necessarily match the requested width/height (for example if the icon loads a
png which simply isn't available in the requested size) this can badly mess up
the layout and even end up in a loop reloading the image.
sizes in Layouts should always be set using
Layout.preferredWidth/preferredHeight.
--
Your team Ubuntu Document Viewer Developers is requested to review the proposed
merge of lp:~mzanetti/ubuntu-docviewer-app/fix-layouts-with-icons into
lp:ubuntu-docviewer-app.
=== modified file 'src/app/qml/documentPage/DocumentGridDelegate.qml'
--- src/app/qml/documentPage/DocumentGridDelegate.qml 2015-03-04 19:38:48 +
+++ src/app/qml/documentPage/DocumentGridDelegate.qml 2015-03-25 11:45:29 +
@@ -79,7 +79,8 @@
anchors.centerIn: parent
anchors.verticalCenterOffset: - infoColumn.height * 0.3
-width: units.gu(8); height: width
+Layout.preferredWidth: units.gu(8);
+Layout.preferredHeight: width
// At the moment the suru icon theme doesn't have much icons.
// Just some note for the future:
=== modified file 'src/app/qml/documentPage/DocumentListDelegate.qml'
--- src/app/qml/documentPage/DocumentListDelegate.qml 2015-03-04 17:44:36 +
+++ src/app/qml/documentPage/DocumentListDelegate.qml 2015-03-25 11:45:29 +
@@ -59,8 +59,8 @@
spacing: units.gu(2)
Icon {
-width: height
-height: units.gu(5)
+Layout.preferredWidth: height
+Layout.preferredHeight: units.gu(5)
// At the moment the suru icon theme doesn't have much icons.
name: {
--
Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers
Post to : ubuntu-touch-coreapps-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers
More help : https://help.launchpad.net/ListHelp
