Unity 8

Merge lp:~aacid/unity8/fix_cropped_image_binding_loop into lp:unity8

fix_cropped_image_binding_loop
Merge into trunk

Proposed by Albert Astals Cid on 2015-10-21

Status:	Merged
Approved by:	Gerry Boland on 2015-10-22
Approved revision:	2006
Merged at revision:	2034
Proposed branch:	lp:~aacid/unity8/fix_cropped_image_binding_loop
Merge into:	lp:unity8
Diff against target:	32 lines (+15/-4) 1 file modified plugins/Dash/CroppedImageMinimumSourceSize.qml (+15/-4)
To merge this branch:	bzr merge lp:~aacid/unity8/fix_cropped_image_binding_loop
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-11-02
Gerry Boland			Approve on 2015-10-22
Michał Sawicz	meh, ohkay	2015-10-21	Approve on 2015-10-21
Review via email: mp+275199@code.launchpad.net

Commit Message

CroppedImageMinimumSourceSize: Fix 'Binding loop detected for property "imageAspectRatio"'

Description of the Change

* Are there any related MPs required for this MP to build/function as expected?
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Gerry Boland (gerboland) wrote on 2015-10-21:

Think a comment explaining "why not bindings?" would do no harm.

Because I'm sure if I saw this code in a year's time, I'd say "I'll use bindings to make it better!" :)

Michał Sawicz (saviq) on 2015-10-21:

review: Approve (meh, ohkay)

PS Jenkins bot (ps-jenkins) wrote on 2015-10-21:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6503/rebuild

review: Needs Fixing (continuous-integration)

lp:~aacid/unity8/fix_cropped_image_binding_loop updated on 2015-10-22

2006. By Albert Astals Cid on 2015-10-22: add commetn

Albert Astals Cid (aacid) wrote on 2015-10-22:

> Think a comment explaining "why not bindings?" would do no harm.
>
> Because I'm sure if I saw this code in a year's time, I'd say "I'll use
> bindings to make it better!" :)

Comment added

PS Jenkins bot (ps-jenkins) wrote on 2015-10-22:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6509/rebuild

review: Needs Fixing (continuous-integration)

Gerry Boland (gerboland) wrote on 2015-10-22:

Will do

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-10-27:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6564/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-11-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6620/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

 === modified file 'plugins/Dash/CroppedImageMinimumSourceSize.qml'
 --- plugins/Dash/CroppedImageMinimumSourceSize.qml	2015-08-31 13:29:42 +0000
 +++ plugins/Dash/CroppedImageMinimumSourceSize.qml	2015-10-22 08:13:17 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2014 Canonical, Ltd.
++ * Copyright (C) 2014, 2015 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
@@ -21,7 +21,18 @@
      fillMode: Image.PreserveAspectCrop
--    readonly property real itemAspectRatio: width / height
--    readonly property real imageAspectRatio: implicitWidth / implicitHeight
--    sourceSize: (imageAspectRatio > itemAspectRatio) ? Qt.size(0, height) : Qt.size(width, 0)
++    property bool useHeight: false
++    function updateUseHeight()
++    {
++        // Do not turn into a binding since otherwise the qml
++        // engine complains about binding loops
++        useHeight = (implicitWidth / implicitHeight) > (width / height);
++    }
++
++    onHeightChanged: updateUseHeight();
++    onWidthChanged: updateUseHeight();
++    onImplicitHeightChanged: updateUseHeight();
++    onImplicitWidthChanged: updateUseHeight();
++
++    sourceSize: useHeight ? Qt.size(0, height) : Qt.size(width, 0)
+ }