Unity 8

Merge lp:~cimi/unity8/preview-sharing-fix-broken-binding into lp:unity8

preview-sharing-fix-broken-binding
Merge into trunk

Proposed by Andrea Cimitan on 2015-12-17

Status:	Merged
Approved by:	Albert Astals Cid on 2016-01-08
Approved revision:	2084
Merged at revision:	2125
Proposed branch:	lp:~cimi/unity8/preview-sharing-fix-broken-binding
Merge into:	lp:unity8
Prerequisite:	lp:~stolowski/unity8/shareData-rename
Diff against target:	160 lines (+74/-16) 5 files modified qml/Dash/Previews/PreviewMediaToolbar.qml (+2/-1) qml/Dash/Previews/PreviewSharing.qml (+2/-3) tests/qmltests/CMakeLists.txt (+1/-0) tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml (+69/-0) tests/qmltests/Dash/Previews/tst_PreviewSharing.qml (+0/-12)
To merge this branch:	bzr merge lp:~cimi/unity8/preview-sharing-fix-broken-binding
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)		2015-12-17	Approve on 2016-01-08
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-01-05
Review via email: mp+280875@code.launchpad.net

Commit Message

Fix a broken binding

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.
from the prerequisite
* Did you perform an exploratory manual test run of your code change and any related functionality?
y
* Did you make sure that your branch does not contain spurious tags?
y
* 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

PS Jenkins bot (ps-jenkins) wrote on 2015-12-17:

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

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2015-12-18:

Is the problem here is that the visible of PreviewSharing got overwritten when some of the parents changed visibility?

Maybe you can write a test that shows it works now?

review: Needs Information

Albert Astals Cid (aacid) wrote on 2015-12-18:

"Maybe you can write a test that shows it works now?" -> i.e. a test that fails with this patch and works with it

lp:~cimi/unity8/preview-sharing-fix-broken-binding updated on 2016-01-04

2083. By Andrea Cimitan on 2016-01-04: Added test

Andrea Cimitan (cimi) wrote on 2016-01-04:

> "Maybe you can write a test that shows it works now?" -> i.e. a test that
> fails with this patch and works with it
added

PS Jenkins bot (ps-jenkins) wrote on 2016-01-04:

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

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2016-01-05:

23/23 Test #22: Whitespace .......................***Failed 1.04 sec
/tmp/buildd/unity8-8.11+16.04.20151208.1bzr2084pkg0vivid1714/tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml: multiple new lines at end of file

review: Needs Fixing

lp:~cimi/unity8/preview-sharing-fix-broken-binding updated on 2016-01-05

2084. By Andrea Cimitan on 2016-01-05: fix whitespace

Andrea Cimitan (cimi) wrote on 2016-01-05:

> 23/23 Test #22: Whitespace .......................***Failed 1.04 sec
> /tmp/buildd/unity8-8.11+16.04.20151208.1bzr2084pkg0vivid1714/tests/qmltests/Da
> sh/Previews/tst_PreviewMediaToolbar.qml: multiple new lines at end of file
fixed

PS Jenkins bot (ps-jenkins) wrote on 2016-01-05:

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

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2016-01-08:

Code looks good and tests pass.

Is there a way this can be tested in real life or no scope uses this yet?

review: Needs Information

Albert Astals Cid (aacid) wrote on 2016-01-08:

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

* Did CI run pass?
Didn't regress

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

review: Approve

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:

Andrea Cimitan

Mohammad Rakibul Hasan

Unity Team

 === modified file 'qml/Dash/Previews/PreviewMediaToolbar.qml'
 --- qml/Dash/Previews/PreviewMediaToolbar.qml	2015-10-26 18:32:19 +0000
 +++ qml/Dash/Previews/PreviewMediaToolbar.qml	2016-01-05 09:52:40 +0000
@@ -23,10 +23,11 @@
      implicitHeight: units.gu(6)
      color: Qt.rgba(0, 0, 0, 0.8)
--    visible: sharingWidget.visible
++    visible: sharingWidget.active
      PreviewSharing {
          id: sharingWidget
++        objectName: "sharingWidget"
          anchors {
              left: parent.left
              verticalCenter: parent.verticalCenter
 === modified file 'qml/Dash/Previews/PreviewSharing.qml'
 --- qml/Dash/Previews/PreviewSharing.qml	2016-01-05 09:52:40 +0000
 +++ qml/Dash/Previews/PreviewSharing.qml	2016-01-05 09:52:40 +0000
@@ -24,13 +24,12 @@
      implicitWidth: button.width
      property var shareData
++    property alias active: peerPicker.active
      readonly property bool isUrlExternal: url && url.indexOf("file:///") != 0 && url.indexOf("/") != 0
      readonly property string contentType: shareData ? shareData["content-type"] : ""
      readonly property var url: shareData ? shareData["uri"] : ""
      readonly property Item rootItem: QuickUtils.rootItem(root)
--    visible: url != ""
--
      AbstractButton {
          id: button
          height: units.gu(4)
@@ -98,7 +97,7 @@
          parent: rootItem
          anchors.fill: parent
          visible: false
--        active: root.visible
++        active: root.url != ""
          sourceComponent: contentPeerComponent
+     }
 === modified file 'tests/qmltests/CMakeLists.txt'
 --- tests/qmltests/CMakeLists.txt	2015-11-26 16:36:53 +0000
 +++ tests/qmltests/CMakeLists.txt	2016-01-05 09:52:40 +0000
@@ -38,6 +38,7 @@
  add_unity8_qmltest(Dash/Previews PreviewHeader)
  add_unity8_qmltest(Dash/Previews PreviewIconActions)
  add_unity8_qmltest(Dash/Previews PreviewImageGallery)
++add_unity8_qmltest(Dash/Previews PreviewMediaToolbar)
  add_unity8_qmltest(Dash/Previews PreviewPayments)
  add_unity8_qmltest(Dash/Previews PreviewProgress)
  add_unity8_qmltest(Dash/Previews PreviewRatingDisplay)
 === added file 'tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml'
 --- tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml	1970-01-01 00:00:00 +0000
 +++ tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml	2016-01-05 09:52:40 +0000
@@ -0,0 +1,69 @@
++/*
++ * Copyright 2014 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
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.4
++import QtTest 1.0
++import Ubuntu.Components 1.3
++import "../../../../qml/Dash/Previews"
++import Unity.Test 0.1 as UT
++
++Rectangle {
++    id: root
++    width: units.gu(40)
++    height: units.gu(80)
++    color: "black"
++
++    property var shareData: {
++        "uri": [
++                    "Text here",
++                    "text here 2",
++                    "text here 3"
++                ],
++        "content-type": "text"
++    }
++
++    property var shareDataNoUri: {
++        "uri": "",
++        "content-type": "text"
++    }
++
++
++    PreviewMediaToolbar {
++        id: previewMediaToolbar
++        anchors { left: parent.left; bottom: parent.bottom; }
++        shareData: root.shareData
++    }
++
++    UT.UnityTestCase {
++        name: "PreviewMediaToolbarTest"
++        when: windowShown
++
++        property Item sharingWidget: findChild(previewMediaToolbar, "sharingWidget")
++
++        function cleanup() {
++            previewMediaToolbar.shareData = root.shareData;
++        }
++
++        function test_visible() {
++            previewMediaToolbar.shareData = shareDataNoUri;
++            compare(previewMediaToolbar.visible, false);
++            compare(sharingWidget.visible, false);
++            previewMediaToolbar.shareData = shareData;
++            compare(previewMediaToolbar.visible, true);
++            compare(sharingWidget.visible, true);
++        }
++    }
++}
 === modified file 'tests/qmltests/Dash/Previews/tst_PreviewSharing.qml'
 --- tests/qmltests/Dash/Previews/tst_PreviewSharing.qml	2016-01-05 09:52:40 +0000
 +++ tests/qmltests/Dash/Previews/tst_PreviewSharing.qml	2016-01-05 09:52:40 +0000
@@ -35,11 +35,6 @@
          "content-type": "text"
+     }
--    property var shareDataNoUri: {
--        "uri": "",
--        "content-type": "text"
--    }
--
      PreviewSharing {
          id: previewSharing
          anchors { left: parent.left; bottom: parent.bottom; }
@@ -54,13 +49,6 @@
          function cleanup() {
              peerPicker.visible = false;
--            previewSharing.shareData = root.shareData;
--        }
--
--        function test_visible() {
--            compare(previewSharing.visible, true);
--            previewSharing.shareData = shareDataNoUri;
--            compare(previewSharing.visible, false);
+         }
          function test_open_picker() {