Merge lp:~nick-dedekind/ubuntu-settings-components/transfer-menu into lp:~registry/ubuntu-settings-components/trunk

Proposed by Nick Dedekind on 2014-06-26
Status: Merged
Approved by: Michał Sawicz on 2014-07-07
Approved revision: 74
Merged at revision: 74
Proposed branch: lp:~nick-dedekind/ubuntu-settings-components/transfer-menu
Merge into: lp:~registry/ubuntu-settings-components/trunk
Prerequisite: lp:~nick-dedekind/ubuntu-settings-components/menu.plugin
Diff against target: 374 lines (+304/-1)
8 files modified
SettingsComponents.qml (+19/-0)
Ubuntu/Settings/Menus/CMakeLists.txt (+1/-0)
Ubuntu/Settings/Menus/TransferMenu.qml (+96/-0)
Ubuntu/Settings/Menus/plugin.cpp (+6/-0)
Ubuntu/Settings/Menus/types.h (+43/-0)
debian/changelog (+2/-1)
tests/qmltests/CMakeLists.txt (+1/-0)
tests/qmltests/Menus/tst_TransferMenu.qml (+136/-0)
To merge this branch: bzr merge lp:~nick-dedekind/ubuntu-settings-components/transfer-menu
Reviewer Review Type Date Requested Status
Michał Sawicz (community) Approve on 2014-07-07
PS Jenkins bot (community) continuous-integration Approve on 2014-07-05
Andrea Cimitan (community) 2014-06-26 Needs Fixing on 2014-07-04
Review via email: mp+224672@code.launchpad.net

Commit message

Added TransferMenu

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

To post a comment you must log in.
Charles Kerr (charlesk) wrote :

I've asked Cimi to review this as he's a member of ubuntu-settings-components and I'm not; however, FWIW:

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

Yes.

Manual test conducted in conjunction with
https://code.launchpad.net/~nick-dedekind/ubuntu-settings-components/transfer-menu/+merge/224672 and https://code.launchpad.net/~charlesk/indicator-transfer/per-transfer-actions/+merge/224537. The manual tests in the indicator-transfer MP put these new transfer menu components on display.

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

Yes.

Andrea Cimitan (cimi) wrote :

I'd test what should change with active: true or false (visibility).

review: Needs Fixing
Nick Dedekind (nick-dedekind) wrote :

> I'd test what should change with active: true or false (visibility).

Done. & added stateText test

Michał Sawicz (saviq) wrote :

Bump changelog so we can depend on it?

More inline.

review: Needs Fixing
Michał Sawicz (saviq) wrote :

Oh and one more thing... Do (should) the enum values come from somewhere?

Nick Dedekind (nick-dedekind) wrote :

Fixed review comments

Nick Dedekind (nick-dedekind) wrote :

> Oh and one more thing... Do (should) the enum values come from somewhere?

They come from indicator-transfer
http://bazaar.launchpad.net/~charlesk/indicator-transfer/per-transfer-actions/view/head:/include/transfer/transfer.h

would need to do some shizzle in there if we wanted to export to includes.

Michał Sawicz (saviq) wrote :

> > Oh and one more thing... Do (should) the enum values come from somewhere?
>
> They come from indicator-transfer
> http://bazaar.launchpad.net/~charlesk/indicator-transfer/per-transfer-
> actions/view/head:/include/transfer/transfer.h
>
> would need to do some shizzle in there if we wanted to export to includes.

I'd just be worried that this get out of sync... There's probably no lib from the indicator that we could link to?

review: Needs Information
Nick Dedekind (nick-dedekind) wrote :

> > > Oh and one more thing... Do (should) the enum values come from somewhere?
> >
> > They come from indicator-transfer
> > http://bazaar.launchpad.net/~charlesk/indicator-transfer/per-transfer-
> > actions/view/head:/include/transfer/transfer.h
> >
> > would need to do some shizzle in there if we wanted to export to includes.
>
> I'd just be worried that this get out of sync... There's probably no lib from
> the indicator that we could link to?

Not for the transfer indicator specifically. There's libindicator, but I don't think it's used for the indicators we use.

Michał Sawicz (saviq) wrote :

Yup.

review: Approve
75. By Nick Dedekind on 2014-07-07

updated debian packaging

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'SettingsComponents.qml'
2--- SettingsComponents.qml 2014-07-01 12:51:28 +0000
3+++ SettingsComponents.qml 2014-07-07 15:54:19 +0000
4@@ -159,6 +159,25 @@
5 onPlay: { mediaPlayer.running = !mediaPlayer.running; }
6 }
7
8+ TransferMenu {
9+ text: "Video Downloading"
10+ stateText: "3 minutes remaning"
11+ iconSource: "tests/artwork/speak-now.jpg"
12+ progress: 0.6
13+ active: true
14+ removable: true
15+ confirmRemoval: true
16+ }
17+
18+ TransferMenu {
19+ text: "Video Downloading"
20+ iconSource: "tests/artwork/speak-now.jpg"
21+ progress: 0.6
22+ active: false
23+ removable: true
24+ confirmRemoval: true
25+ }
26+
27 AccessPointMenu {
28 checked: true
29 secure: true
30
31=== modified file 'Ubuntu/Settings/Menus/CMakeLists.txt'
32--- Ubuntu/Settings/Menus/CMakeLists.txt 2014-07-07 15:54:19 +0000
33+++ Ubuntu/Settings/Menus/CMakeLists.txt 2014-07-07 15:54:19 +0000
34@@ -11,6 +11,7 @@
35
36 set(UbuntuSettingsMenusQml_SOURCES
37 plugin.cpp
38+ types.h
39 )
40 add_definitions(-DUBUNTUSETTINGSCOMPONENTS_LIBRARY)
41
42
43=== added file 'Ubuntu/Settings/Menus/TransferMenu.qml'
44--- Ubuntu/Settings/Menus/TransferMenu.qml 1970-01-01 00:00:00 +0000
45+++ Ubuntu/Settings/Menus/TransferMenu.qml 2014-07-07 15:54:19 +0000
46@@ -0,0 +1,96 @@
47+/*
48+ * Copyright 2014 Canonical Ltd.
49+ *
50+ * This program is free software; you can redistribute it and/or modify
51+ * it under the terms of the GNU Lesser General Public License as published by
52+ * the Free Software Foundation; version 3.
53+ *
54+ * This program is distributed in the hope that it will be useful,
55+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
56+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
57+ * GNU Lesser General Public License for more details.
58+ *
59+ * You should have received a copy of the GNU Lesser General Public License
60+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
61+ */
62+
63+import QtQuick 2.0
64+import Ubuntu.Components 0.1
65+import Ubuntu.Components.ListItems 0.1 as ListItem
66+import QtQuick.Layouts 1.1
67+
68+ListItem.Empty {
69+ id: menu
70+
71+ property alias iconSource: icon.source
72+ property alias text: label.text
73+ property alias stateText: stateLabel.text
74+ property alias progress: progressBar.value
75+ property bool active: false
76+
77+ property alias maximum: progressBar.maximumValue
78+
79+ implicitHeight: row.height + units.gu(2)
80+
81+ RowLayout {
82+ id: row
83+ anchors {
84+ left: parent.left
85+ right: parent.right
86+ verticalCenter: parent.verticalCenter
87+ leftMargin: menu.__contentsMargins
88+ rightMargin: menu.__contentsMargins
89+ }
90+
91+ UbuntuShape {
92+ id: imageShape
93+ Layout.preferredWidth: units.gu(5)
94+ Layout.preferredHeight: units.gu(5)
95+
96+ Layout.alignment: Qt.AlignTop
97+
98+ image: Image {
99+ objectName: "icon"
100+ id: icon
101+
102+ sourceSize {
103+ width: units.gu(5)
104+ height: units.gu(5)
105+ }
106+ }
107+ }
108+
109+ ColumnLayout {
110+ spacing: units.gu(0.5)
111+
112+ Label {
113+ id: label
114+ objectName: "text"
115+ Layout.fillWidth: true
116+
117+ elide: Text.ElideRight
118+ maximumLineCount: 1
119+ }
120+
121+ ProgressBar {
122+ id: progressBar
123+ objectName: "progress"
124+ visible: menu.active
125+ value: 0.0
126+
127+ Layout.preferredHeight: units.gu(2)
128+ Layout.fillWidth: true
129+ }
130+
131+ Label {
132+ id: stateLabel
133+ objectName: "stateText"
134+ Layout.fillWidth: true
135+ visible: menu.active
136+
137+ elide: Text.ElideRight
138+ maximumLineCount: 1
139+ }
140+ }
141+ }
142+}
143
144=== modified file 'Ubuntu/Settings/Menus/plugin.cpp'
145--- Ubuntu/Settings/Menus/plugin.cpp 2014-07-07 15:54:19 +0000
146+++ Ubuntu/Settings/Menus/plugin.cpp 2014-07-07 15:54:19 +0000
147@@ -14,8 +14,14 @@
148 * along with this program. If not, see <http://www.gnu.org/licenses/>.
149 */
150
151+// local
152 #include "plugin.h"
153+#include "types.h"
154+
155+// Qt
156+#include <QtQml/qqml.h>
157
158 void UbuntuSettingsComponentsPlugin::registerTypes(const char *uri)
159 {
160+ qmlRegisterUncreatableType<TransferState>(uri, 0, 1, "TransferState", "Can't create TransferState class");
161 }
162
163=== added file 'Ubuntu/Settings/Menus/types.h'
164--- Ubuntu/Settings/Menus/types.h 1970-01-01 00:00:00 +0000
165+++ Ubuntu/Settings/Menus/types.h 2014-07-07 15:54:19 +0000
166@@ -0,0 +1,43 @@
167+/*
168+ * Copyright (C) 2014 Canonical, Ltd.
169+ *
170+ * This program is free software; you can redistribute it and/or modify
171+ * it under the terms of the GNU General Public License as published by
172+ * the Free Software Foundation; version 3.
173+ *
174+ * This program is distributed in the hope that it will be useful,
175+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
176+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
177+ * GNU General Public License for more details.
178+ *
179+ * You should have received a copy of the GNU General Public License
180+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
181+ */
182+
183+#ifndef UBUNTUSETTINGSMENUSTYPES_H
184+#define UBUNTUSETTINGSMENUSTYPES_H
185+
186+#include "pluginglobal.h"
187+
188+#include <QObject>
189+
190+class UBUNTUSETTINGSCOMPONENTS_EXPORT TransferState : public QObject
191+{
192+ Q_OBJECT
193+ Q_ENUMS(TransferStates)
194+public:
195+ enum TransferStates {
196+ Queued,
197+ Running,
198+ Paused,
199+ Canceled,
200+ Hashing,
201+ Processing,
202+ Finished,
203+ Error
204+ };
205+
206+ TransferState(QObject* parent = 0): QObject(parent) {}
207+};
208+
209+#endif // UBUNTUSETTINGSMENUSTYPES_H
210
211=== modified file 'debian/changelog'
212--- debian/changelog 2014-07-07 15:54:19 +0000
213+++ debian/changelog 2014-07-07 15:54:19 +0000
214@@ -2,8 +2,9 @@
215
216 [ Nick Dedekind ]
217 * Added plugin module for Ubuntu.Settings.Menus
218+ * Added TransferMenu
219
220- -- Nicholas Dedekind <nicholas.dedekind@gmail.com> Mon, 07 Jul 2014 16:43:16 +0100
221+ -- Nicholas Dedekind <nicholas.dedekind@gmail.com> Mon, 07 Jul 2014 12:28:37 +0100
222
223 ubuntu-settings-components (0.2+14.10.20140701.2-0ubuntu2) utopic; urgency=medium
224
225
226=== modified file 'tests/qmltests/CMakeLists.txt'
227--- tests/qmltests/CMakeLists.txt 2014-07-07 15:54:19 +0000
228+++ tests/qmltests/CMakeLists.txt 2014-07-07 15:54:19 +0000
229@@ -31,3 +31,4 @@
230 add_qml_test(Menus UserSessionMenu)
231 add_qml_test(Menus TextMessageMenu)
232 add_qml_test(Menus TimeZoneMenu)
233+add_qml_test(Menus TransferMenu)
234
235=== added file 'tests/qmltests/Menus/tst_TransferMenu.qml'
236--- tests/qmltests/Menus/tst_TransferMenu.qml 1970-01-01 00:00:00 +0000
237+++ tests/qmltests/Menus/tst_TransferMenu.qml 2014-07-07 15:54:19 +0000
238@@ -0,0 +1,136 @@
239+/*
240+ * Copyright 2014 Canonical Ltd.
241+ *
242+ * This program is free software; you can redistribute it and/or modify
243+ * it under the terms of the GNU Lesser General Public License as published by
244+ * the Free Software Foundation; version 3.
245+ *
246+ * This program is distributed in the hope that it will be useful,
247+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
248+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
249+ * GNU Lesser General Public License for more details.
250+ *
251+ * You should have received a copy of the GNU Lesser General Public License
252+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
253+ */
254+
255+import QtQuick 2.0
256+import QtTest 1.0
257+import Ubuntu.Components 0.1
258+import Ubuntu.Settings.Menus 0.1
259+import "../utils.js" as UtilsJS
260+
261+Item {
262+ width: units.gu(42)
263+ height: units.gu(75)
264+
265+ Flickable {
266+ id: flickable
267+
268+ anchors.fill: parent
269+ contentWidth: column.width
270+ contentHeight: column.height
271+
272+ Item {
273+ id: column
274+
275+ width: flickable.width
276+ height: childrenRect.height
277+
278+ TransferMenu {
279+ id: transferMenu
280+
281+ text: "Downloading Movie"
282+ progress: 0
283+ active: false
284+ iconSource: Qt.resolvedUrl("../../artwork/avatar.png")
285+ }
286+ TransferMenu {
287+ id: transferMenu2
288+ anchors.top: transferMenu.bottom
289+
290+ text: "Syncing Data"
291+ progress: 0.6
292+ active: true
293+ iconSource: Qt.resolvedUrl("../../artwork/rhythmbox.png")
294+ }
295+ }
296+ }
297+
298+ TestCase {
299+ name: "TransferMenu"
300+ when: windowShown
301+
302+ function init() {
303+ transferMenu.text = "";
304+ transferMenu.iconSource = "";
305+ transferMenu.progress = 0;
306+ transferMenu.active = false;
307+ }
308+
309+ function test_iconSource_data() {
310+ return [ { icon: Qt.resolvedUrl("../../artwork/avatar.png") },
311+ { icon: Qt.resolvedUrl("../../artwork/rhythmbox.png") }
312+ ];
313+ }
314+
315+ function test_iconSource(data) {
316+ transferMenu.iconSource = data.icon;
317+
318+ var icon = UtilsJS.findChild(transferMenu, "icon");
319+ compare(icon.source, data.icon, "Icon does not match data");
320+ }
321+
322+ function test_text_data() {
323+ return [ { text: "Text 1" },
324+ { text: "Text 2" }
325+ ];
326+ }
327+
328+ function test_text(data) {
329+ transferMenu.text = data.text;
330+
331+ var text = UtilsJS.findChild(transferMenu, "text");
332+ compare(text.text, data.text, "Text does not match data");
333+ }
334+
335+ function test_stateText_data() {
336+ return [ { stateText: "State 1" },
337+ { stateText: "State 2" }
338+ ];
339+ }
340+
341+ function test_stateText(data) {
342+ transferMenu.stateText = data.stateText;
343+
344+ var stateText = UtilsJS.findChild(transferMenu, "stateText");
345+ compare(stateText.text, data.stateText, "State text does not match data");
346+ }
347+
348+ function test_progress_data() {
349+ return [ { progress: 0.5 },
350+ { progress: 1.0 }
351+ ];
352+ }
353+
354+ function test_progress(data) {
355+ transferMenu.progress = data.progress;
356+
357+ var progress = UtilsJS.findChild(transferMenu, "progress");
358+ compare(progress.value, data.progress, "Progress does not match expected value");
359+ }
360+
361+ function test_active() {
362+ var progress = UtilsJS.findChild(transferMenu, "progress");
363+ var stateText = UtilsJS.findChild(transferMenu, "stateText");
364+
365+ transferMenu.active = true;
366+ compare(progress.visible, true, "Progress should be visible when active");
367+ compare(stateText.visible, true, "State should be visible when active");
368+
369+ transferMenu.active = false;
370+ compare(progress.visible, false, "Progress should not be visible when inactive");
371+ compare(stateText.visible, false, "State should not be visible when inactive");
372+ }
373+ }
374+}

Subscribers

People subscribed via source and target branches

to all changes: