Unity 8

Merge lp:~cimi/unity8/wide-preview-zoomable-videoplayback into lp:unity8

wide-preview-zoomable-videoplayback
Merge into trunk

Proposed by Andrea Cimitan on 2015-10-26

Status:	Merged
Approved by:	Albert Astals Cid on 2015-10-26
Approved revision:	1997
Merged at revision:	2024
Proposed branch:	lp:~cimi/unity8/wide-preview-zoomable-videoplayback
Merge into:	lp:unity8
Prerequisite:	lp:~cimi/unity8/sdk1.3_newUbuntuShape
Diff against target:	202 lines (+45/-21) 8 files modified qml/Components/LazyImage.qml (+23/-10) qml/Dash/Previews/Preview.qml (+2/-2) qml/Dash/Previews/PreviewVideoPlayback.qml (+3/-0) qml/Dash/Previews/PreviewWidget.qml (+3/-0) qml/Dash/Previews/PreviewWidgetFactory.qml (+3/-0) qml/Dash/Previews/PreviewZoomableImage.qml (+8/-6) tests/qmltests/Dash/Previews/tst_PreviewExpandable.qml (+2/-2) tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml (+1/-1)
To merge this branch:	bzr merge lp:~cimi/unity8/wide-preview-zoomable-videoplayback
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration	2015-10-26	Needs Fixing on 2015-10-30
Albert Astals Cid (community)		2015-10-26	Approve on 2015-10-26
Review via email: mp+275696@code.launchpad.net

This proposal supersedes a proposal from 2015-10-01.

Commit Message

Make zoomable and video playback widgets edge to edge

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.
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?
yes

PS Jenkins bot (ps-jenkins) wrote on 2015-10-01: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-10-01: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-10-02: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2015-10-14: Posted in a previous version of this proposal

What's the purpose of itemComponent? Is it to not load the ubuntushape? Maybe we should just set the active property to false instead of creating a dummy Item?

review: Needs Information

Albert Astals Cid (aacid) wrote on 2015-10-14: Posted in a previous version of this proposal

Why units.gu(35) for the implicitWidth of the image, does it make any difference given we're anchoring to the parent left and right when using it anyway?

review: Needs Information

Albert Astals Cid (aacid) wrote on 2015-10-14: Posted in a previous version of this proposal

the -2 in the margins looks a bit weird, is that to counterbalance the -1 in the PreviewWidgetFactory of Preview.qml?

Would it make more sense to actually have a property in the widgets saying if they want those margins than doing a -2 that will break if we change those -1 to -3 or -1.5 at some point?

review: Needs Information

PS Jenkins bot (ps-jenkins) wrote on 2015-10-19: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2015-10-20: Posted in a previous version of this proposal

You don't use ubuntuShapeComponent anymore, right? Remove it?

review: Needs Information

Albert Astals Cid (aacid) wrote on 2015-10-20: Posted in a previous version of this proposal

Having the widgetMargins to be -units.gu(1) still is suboptimal, means the widget knows about its parent and is basically not reusable elsewhere.

But i don't see an elegant way to "fix" it :/

You forgot to comment on this "Why units.gu(35) for the implicitWidth of the image, does it make any difference given we're anchoring to the parent left and right when using it anyway?"

review: Needs Information

Albert Astals Cid (aacid) wrote on 2015-10-23: Posted in a previous version of this proposal

The questions i have are more design than anything so let's get it out there and see if there's complains

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

* Did CI run pass? If not, please explain why.
enough

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

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-10-23: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

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

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

review: Needs Fixing (continuous-integration)

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

make tryLazyImage
gives me
file:///home/tsdgeos_work/phablet/unity8/wide-preview-zoomable-videoplayback/qml/Components/LazyImage.qml:96: Unable to assign [undefined] to bool

review: Needs Fixing

lp:~cimi/unity8/wide-preview-zoomable-videoplayback updated on 2015-10-26

1997. By Andrea Cimitan on 2015-10-26: Fix for review

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

ok, still good

review: Approve

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

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

review: Needs Fixing (continuous-integration)

lp:~cimi/unity8/wide-preview-zoomable-videoplayback updated on 2015-10-27

1998. By Andrea Cimitan on 2015-10-27: [ Albert Astals Cid ]
* Clazy fixes
* Enable Efficient String Construction by default
[ CI Train Bot ]
* New rebuild forced.
[ Daniel d'Andrada ]
* Have unity8 drawing its own cursor (LP: #1488417)
* Initial multi-monitor support
[ Daniel van Vugt ]
* Disable Qt's stuttering 'touch compression' to fix scrolling
smoothness (LP: #1486341, #1488327)
[ Lukáš Tinkl ]
* Fix autopilot wizard test skipping the reporting page
* Implement Unity.Platform plugin wrapping org.freedesktop.hostname1
(LP: #1504318)
* React to window title (aka surface name) changes (LP: #1497092)
* Rotate the screenshots according to the actual orientation
[ Michał Sawicz ]
* Fix application API dependency
* Have unity8 drawing its own cursor (LP: #1488417)
* Initial multi-monitor support
* Rotate the screenshots according to the actual orientation
[ Nick Dedekind ]
* Fixed leak in UnityMenuModelStackTest
* Moved time translation to SDK (LP: #1372061)

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/6561/rebuild

review: Needs Fixing (continuous-integration)

lp:~cimi/unity8/wide-preview-zoomable-videoplayback updated on 2015-10-28

1999. By Andrea Cimitan on 2015-10-28: Do not show ubuntuShape framed lazyimage when not requested

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6600/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:

Andrea Cimitan

Mohammad Rakibul Hasan

Unity Team

1	=== modified file 'qml/Components/LazyImage.qml'
2	--- qml/Components/LazyImage.qml 2015-10-28 18:11:37 +0000
3	+++ qml/Components/LazyImage.qml 2015-10-28 18:11:37 +0000
4	@@ -34,6 +34,8 @@
5	property alias asynchronous: image.asynchronous
6	property alias cache: image.cache
7	property alias sourceImage: image
8	+
9	+ property bool useUbuntuShape: true
10	property bool pressed: false
11
12	state: "default"
13	@@ -47,12 +49,15 @@
14	}
15	}
16
17	- UbuntuShape {
18	+ Loader {
19	id: placeholder
20	objectName: "placeholder"
21	- backgroundColor: "#22FFFFFF"
22	anchors.fill: shape
23	+ active: useUbuntuShape
24	visible: opacity != 0
25	+ sourceComponent: UbuntuShape {
26	+ backgroundColor: "#22FFFFFF"
27	+ }
28
29	ActivityIndicator {
30	id: activity
31	@@ -75,27 +80,35 @@
32	}
33	}
34
35	- UbuntuShapeOverlay {
36	+ Loader {
37	id: shape
38	objectName: "shape"
39	height: root.initialHeight
40	width: root.initialWidth
41	anchors.centerIn: root.scaleTo == "fit" ? parent : undefined
42	-
43	+ active: useUbuntuShape
44	opacity: 0
45	visible: opacity != 0
46	- overlayColor: Qt.rgba(0, 0, 0, root.pressed ? 0.1 : 0)
47	- overlayRect: Qt.rect(0.0, 0.0, 1.0, 1.0)
48	- sourceFillMode: UbuntuShape.PreserveAspectCrop
49	- sourceHorizontalAlignment: UbuntuShape.AlignHCenter
50	- sourceVerticalAlignment: UbuntuShape.AlignVCenter
51	- source: Image {
52	+ sourceComponent: UbuntuShapeOverlay {
53	+ property bool pressed: false
54	+ overlayColor: Qt.rgba(0, 0, 0, pressed ? 0.1 : 0)
55	+ overlayRect: Qt.rect(0.0, 0.0, 1.0, 1.0)
56	+ }
57	+ onLoaded: {
58	+ item.source = image;
59	+ item.pressed = Qt.binding(function() { return root.pressed; });
60	+ }
61	+
62	+ Image {
63	id: image
64	objectName: "image"
65
66	property url nextSource
67	property string format: image.implicitWidth > image.implicitHeight ? "landscape" : "portrait"
68
69	+ anchors.fill: parent
70	+ visible: !useUbuntuShape
71	+ fillMode: Image.PreserveAspectFit
72	asynchronous: true
73	cache: false
74	sourceSize.width: root.scaleTo == "width" ? root.width
75
76	=== modified file 'qml/Dash/Previews/Preview.qml'
77	--- qml/Dash/Previews/Preview.qml 2015-10-28 18:11:37 +0000
78	+++ qml/Dash/Previews/Preview.qml 2015-10-28 18:11:37 +0000
79	@@ -100,8 +100,8 @@
80	anchors {
81	left: parent.left
82	right: parent.right
83	- leftMargin: units.gu(1)
84	- rightMargin: units.gu(1)
85	+ leftMargin: widgetMargins
86	+ rightMargin: widgetMargins
87	}
88
89	onTriggered: {
90
91	=== modified file 'qml/Dash/Previews/PreviewVideoPlayback.qml'
92	--- qml/Dash/Previews/PreviewVideoPlayback.qml 2015-10-28 18:11:37 +0000
93	+++ qml/Dash/Previews/PreviewVideoPlayback.qml 2015-10-28 18:11:37 +0000
94	@@ -30,6 +30,8 @@
95	implicitWidth: units.gu(35)
96	implicitHeight: childrenRect.height
97
98	+ widgetMargins: -units.gu(1)
99	+
100	LazyImage {
101	objectName: "screenshot"
102	anchors {
103	@@ -51,6 +53,7 @@
104	return "";
105	}
106	initialHeight: width * 10 / 16
107	+ useUbuntuShape: false
108
109	Image {
110	objectName: "playButton"
111
112	=== modified file 'qml/Dash/Previews/PreviewWidget.qml'
113	--- qml/Dash/Previews/PreviewWidget.qml 2015-08-31 10:40:59 +0000
114	+++ qml/Dash/Previews/PreviewWidget.qml 2015-10-28 18:11:37 +0000
115	@@ -34,6 +34,9 @@
116	//! Should the widget show in expanded mode (For those that support it)
117	property bool expanded: true
118
119	+ //! Set margins width.
120	+ property real widgetMargins: units.gu(1)
121	+
122	/*! \brief This signal should be emitted when a preview action was triggered.
123	*
124	* \param widgetId, actionId Respective identifiers from widgetData.
125
126	=== modified file 'qml/Dash/Previews/PreviewWidgetFactory.qml'
127	--- qml/Dash/Previews/PreviewWidgetFactory.qml 2015-09-15 12:05:09 +0000
128	+++ qml/Dash/Previews/PreviewWidgetFactory.qml 2015-10-28 18:11:37 +0000
129	@@ -40,6 +40,9 @@
130	//! Set to true if the parent preview is displayed.
131	property bool isCurrentPreview: false
132
133	+ //! Set margins width.
134	+ property real widgetMargins: status === Loader.Ready ? item.widgetMargins : units.gu(1)
135	+
136	//! Triggered signal forwarded from the widgets.
137	signal triggered(string widgetId, string actionId, var data)
138
139
140	=== modified file 'qml/Dash/Previews/PreviewZoomableImage.qml'
141	--- qml/Dash/Previews/PreviewZoomableImage.qml 2015-10-28 18:11:37 +0000
142	+++ qml/Dash/Previews/PreviewZoomableImage.qml 2015-10-28 18:11:37 +0000
143	@@ -27,7 +27,10 @@
144
145	PreviewWidget {
146	id: root
147	- implicitHeight: units.gu(22)
148	+ implicitWidth: units.gu(35)
149	+ implicitHeight: lazyImage.height
150	+
151	+ widgetMargins: -units.gu(1)
152
153	property Item rootItem: QuickUtils.rootItem(root)
154
155	@@ -35,14 +38,13 @@
156	id: lazyImage
157	objectName: "lazyImage"
158	anchors {
159	- top: parent.top
160	- bottom: parent.bottom
161	- horizontalCenter: parent.horizontalCenter
162	+ left: parent.left
163	+ right: parent.right
164	}
165	- scaleTo: "height"
166	+ scaleTo: "width"
167	source: widgetData["source"]
168	asynchronous: true
169	-
170	+ useUbuntuShape: false
171	pressed: mouseArea.pressed
172
173	MouseArea {
174
175	=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewExpandable.qml'
176	--- tests/qmltests/Dash/Previews/tst_PreviewExpandable.qml 2015-10-28 18:11:37 +0000
177	+++ tests/qmltests/Dash/Previews/tst_PreviewExpandable.qml 2015-10-28 18:11:37 +0000
178	@@ -319,9 +319,9 @@
179	previewWidgetFactory.widgetType = allWidgetsModel.get(i).type;
180	previewWidgetFactory.active = true;
181
182	- // Wait for the height ot settle by waiting twice the time of the
183	+ // Wait for the height ot settle by waiting four times the time of the
184	// longest of the height behaviour animations
185	- wait(UbuntuAnimation.SnapDuration * 2);
186	+ wait(UbuntuAnimation.SnapDuration * 4);
187
188	// Check the item inside the expandable has the same height
189	// as the one straight from the factory
190
191	=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml'
192	--- tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml 2015-05-06 15:05:06 +0000
193	+++ tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml 2015-10-28 18:11:37 +0000
194	@@ -41,7 +41,7 @@
195
196	Loader {
197	id: loader
198	- anchors.centerIn: parent
199	+ width: parent.width
200	sourceComponent: PreviewZoomableImage {
201	widgetData: widgetData0
202	}