Merge lp:~uriboni/camera-app/share-multiple-items into lp:camera-app/staging

Proposed by Ugo Riboni
Status: Merged
Approved by: Florian Boucault
Approved revision: 670
Merged at revision: 665
Proposed branch: lp:~uriboni/camera-app/share-multiple-items
Merge into: lp:camera-app/staging
Diff against target: 396 lines (+243/-20)
8 files modified
GalleryView.qml (+1/-3)
PhotogridView.qml (+30/-4)
UnableShareDialog.qml (+34/-0)
camera-app.qml (+1/-1)
debian/control (+3/-0)
tests/autopilot/camera_app/tests/test_gallery_view.py (+85/-11)
tests/unittests/CMakeLists.txt (+1/-1)
tests/unittests/tst_PhotogridView.qml (+88/-0)
To merge this branch: bzr merge lp:~uriboni/camera-app/share-multiple-items
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Florian Boucault (community) Needs Fixing
Review via email: mp+287895@code.launchpad.net

Commit message

Allow sharing multiple files, except if they are mixed content

Description of the change

Allow sharing multiple files, except if they are mixed content

To post a comment you must log in.
Revision history for this message
Florian Boucault (fboucault) wrote :

code in PhotogridView.qml/onTriggered could be written in a clearer way, for example:

onTriggered: {
  if (selectedFilesMixedTypes()) {
    PopupUtils.open(unableShareDialogComponent).parent = photogridView;
  } else {
    PopupUtils.open(sharePopoverComponent).parent = photogridView;
  }
}

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote :

In some cases when selecting 3 photos camera thinks I have mixed type contents. The function selectedFilesMixedTypes() should be unit tested to root out these bugs.

review: Needs Fixing
Revision history for this message
Florian Boucault (fboucault) wrote :

Going to messaging app and trying to share pictures via the camera app: unable to tick more than 1 picture. Without this patch, it works fine.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
663. By Ugo Riboni

Fix and simplify mixed media detection, add unit tests for it

664. By Ugo Riboni

Use single selection mode only in grid view if the content hub explicitly asks for it

665. By Ugo Riboni

Filter the grid view according to the content type requested by content hub

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
666. By Ugo Riboni

Allow unit tests to find c++ plugin when run out of tree

667. By Ugo Riboni

Fix PEP8 issue

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
668. By Ugo Riboni

Add build dependencies needed for unit tests

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
669. By Ugo Riboni

Merge parent branch

670. By Ugo Riboni

Fix flake8 errors

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'GalleryView.qml'
2--- GalleryView.qml 2016-02-23 11:46:52 +0000
3+++ GalleryView.qml 2016-03-10 12:11:22 +0000
4@@ -35,7 +35,7 @@
5 StorageLocations.removableStorageVideosLocation]
6 typeFilters: !main.contentExportMode ? [ "image", "video" ]
7 : [MimeTypeMapper.contentTypeToMimeType(main.transferContentType)]
8- singleSelectionOnly: main.transfer.selectionType === ContentTransfer.Single
9+ singleSelectionOnly: main.contentExportMode && main.transfer.selectionType === ContentTransfer.Single
10 }
11
12 property bool gridMode: main.contentExportMode
13@@ -58,7 +58,6 @@
14
15 function exitUserSelectionMode() {
16 model.clearSelection();
17- model.singleSelectionOnly = true;
18 userSelectionMode = false;
19 }
20
21@@ -97,7 +96,6 @@
22 onPhotoPressAndHold: {
23 if (!galleryView.userSelectionMode) {
24 galleryView.userSelectionMode = true;
25- model.singleSelectionOnly = false;
26 model.toggleSelected(index);
27 }
28 }
29
30=== modified file 'PhotogridView.qml'
31--- PhotogridView.qml 2016-03-03 14:12:26 +0000
32+++ PhotogridView.qml 2016-03-10 12:11:22 +0000
33@@ -40,11 +40,14 @@
34 Action {
35 text: i18n.tr("Share")
36 iconName: "share"
37- enabled: model.selectedFiles.length <= 1
38+ enabled: model.selectedFiles.length > 0
39 onTriggered: {
40- if (model.selectedFiles.length > 0) {
41- var dialog = PopupUtils.open(sharePopoverComponent)
42- dialog.parent = photogridView
43+ // Display a warning message if we are attempting to share mixed
44+ // content, as the framework does not properly support this
45+ if (selectionContainsMixedMedia()) {
46+ PopupUtils.open(unableShareDialogComponent).parent = photogridView;
47+ } else {
48+ PopupUtils.open(sharePopoverComponent).parent = photogridView;
49 }
50 }
51 },
52@@ -60,6 +63,19 @@
53 }
54 ]
55
56+ function selectionContainsMixedMedia() {
57+ var selection = model.selectedFiles;
58+ var lastType = model.get(selection[0], "fileType");
59+ for (var i = 1; i < selection.length; i++) {
60+ var type = model.get(selection[i], "fileType");
61+ if (type !== lastType) {
62+ return true;
63+ }
64+ lastType = type;
65+ }
66+ return false;
67+ }
68+
69 function showPhotoAtIndex(index) {
70 gridView.positionViewAtIndex(index, GridView.Center);
71 }
72@@ -170,6 +186,7 @@
73 visible: inSelectionMode
74
75 Icon {
76+ objectName: "mediaItemCheckBox"
77 anchors.centerIn: parent
78 width: parent.width * 0.8
79 height: parent.height * 0.8
80@@ -235,4 +252,13 @@
81 onVisibleChanged: photogridView.toggleHeader()
82 }
83 }
84+
85+ Component {
86+ id: unableShareDialogComponent
87+ UnableShareDialog {
88+ objectName: "unableShareDialog"
89+ onVisibleChanged: photogridView.toggleHeader()
90+ }
91+ }
92+
93 }
94
95=== added file 'UnableShareDialog.qml'
96--- UnableShareDialog.qml 1970-01-01 00:00:00 +0000
97+++ UnableShareDialog.qml 2016-03-10 12:11:22 +0000
98@@ -0,0 +1,34 @@
99+/*
100+ * Copyright (C) 2016 Canonical Ltd
101+ *
102+ * This program is free software: you can redistribute it and/or modify
103+ * it under the terms of the GNU General Public License version 3 as
104+ * published by the Free Software Foundation.
105+ *
106+ * This program is distributed in the hope that it will be useful,
107+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
108+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
109+ * GNU General Public License for more details.
110+ *
111+ * You should have received a copy of the GNU General Public License
112+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
113+ */
114+
115+import QtQuick 2.4
116+import Ubuntu.Components 1.3
117+import Ubuntu.Components.Popups 1.3
118+
119+Dialog {
120+ id: dialog
121+ objectName: "unableShareDialog"
122+
123+ title: i18n.tr("Unable to share")
124+ text: i18n.tr("Unable to share photos and videos at the same time")
125+
126+ Button {
127+ objectName: "unableShareDialogOk"
128+ text: i18n.tr("Ok")
129+ color: UbuntuColors.orange
130+ onClicked: PopupUtils.close(dialog);
131+ }
132+}
133
134=== modified file 'camera-app.qml'
135--- camera-app.qml 2016-03-08 14:42:51 +0000
136+++ camera-app.qml 2016-03-10 12:11:22 +0000
137@@ -282,7 +282,7 @@
138
139 property bool contentExportMode: transfer !== null
140 property var transfer: null
141- property var transferContentType: ContentType.Pictures
142+ property var transferContentType: transfer ? transfer.contentType : "image"
143
144 function exportContent(urls) {
145 if (!main.transfer) return;
146
147=== modified file 'debian/control'
148--- debian/control 2015-07-31 02:23:54 +0000
149+++ debian/control 2016-03-10 12:11:22 +0000
150@@ -13,11 +13,14 @@
151 qtbase5-dev,
152 qtdeclarative5-dev,
153 qml-module-qtquick2,
154+ qml-module-qtpositioning,
155 qml-module-qttest,
156 qtdeclarative5-ubuntu-ui-toolkit-plugin,
157 qtdeclarative5-unity-action-plugin (>= 1.1.0),
158 qtdeclarative5-usermetrics0.1,
159 qtdeclarative5-ubuntu-content1,
160+ qtdeclarative5-ubuntu-thumbnailer0.1,
161+ qtdeclarative5-ubuntu-ui-extras0.2,
162 qtmultimedia5-dev,
163 libusermetricsinput1-dev,
164 gettext,
165
166=== modified file 'tests/autopilot/camera_app/tests/test_gallery_view.py'
167--- tests/autopilot/camera_app/tests/test_gallery_view.py 2016-03-02 14:35:50 +0000
168+++ tests/autopilot/camera_app/tests/test_gallery_view.py 2016-03-10 12:11:22 +0000
169@@ -35,16 +35,21 @@
170 self.assertThat(slideshow_view.visible, Eventually(Equals(False)))
171 self.assertThat(photogrid_view.visible, Eventually(Equals(True)))
172
173- def select_first_photo(self):
174- # select the first photo
175+ def select_media(self, index=0):
176+ # select the photo with index, default to the first one
177 gallery = self.main_window.get_gallery()
178- photo = gallery.wait_select_single(objectName="mediaItem0")
179- self.pointing_device.move_to_object(photo)
180-
181- # do a long press to enter Multiselection mode
182- self.pointing_device.press()
183- sleep(1)
184- self.pointing_device.release()
185+ photo = gallery.wait_select_single(objectName="mediaItem" + str(index))
186+ checkbox = photo.wait_select_single(objectName="mediaItemCheckBox")
187+
188+ self.pointing_device.move_to_object(checkbox)
189+
190+ if checkbox.visible:
191+ self.click()
192+ else:
193+ # do a long press to enter Multiselection mode
194+ self.pointing_device.press()
195+ sleep(1)
196+ self.pointing_device.release()
197
198
199 class TestCameraGalleryView(CameraAppTestCase, TestCameraGalleryViewMixin):
200@@ -186,7 +191,7 @@
201 def test_delete_photo_from_multiselection(self):
202 self.main_window.swipe_to_gallery(self)
203 self.move_from_slideshow_to_photogrid()
204- self.select_first_photo()
205+ self.select_media()
206
207 # open actions drawer
208 gallery = self.main_window.get_gallery()
209@@ -211,7 +216,7 @@
210 def test_multiselection_mode(self):
211 self.main_window.swipe_to_gallery(self)
212 self.move_from_slideshow_to_photogrid()
213- self.select_first_photo()
214+ self.select_media()
215
216 # exit the multiselection mode
217 gallery = self.main_window.get_gallery()
218@@ -224,3 +229,72 @@
219
220 self.assertThat(slideshow_view.visible, Eventually(Equals(False)))
221 self.assertThat(photogrid_view.visible, Eventually(Equals(True)))
222+
223+
224+class TestCameraGalleryViewWithPhotosAndVideo(
225+ TestCameraGalleryViewMixin, CameraAppTestCase):
226+ """Tests the camera gallery view with two photos and a video"""
227+
228+ def setUp(self):
229+ self.delete_all_media()
230+ self.add_sample_photo()
231+ self.add_sample_video()
232+
233+ super(TestCameraGalleryViewWithPhotosAndVideo, self).setUp()
234+ self.assertThat(
235+ self.main_window.get_qml_view().visible, Eventually(Equals(True)))
236+
237+ def tearDown(self):
238+ super(TestCameraGalleryViewWithPhotosAndVideo, self).tearDown()
239+
240+ def verify_share_state(self, expectedState, close=True):
241+ # open actions drawer
242+ gallery = self.main_window.get_gallery()
243+ opt = gallery.wait_select_single(objectName="additionalActionsButton")
244+ self.pointing_device.move_to_object(opt)
245+ self.pointing_device.click()
246+
247+ # verify expected state
248+ share = gallery.wait_select_single(objectName="actionButtonShare")
249+ self.assertThat(share.enabled, Eventually(Equals(expectedState)))
250+
251+ if (close):
252+ # close actions drawer
253+ self.pointing_device.move_to_object(opt)
254+ self.pointing_device.click()
255+ else:
256+ return share
257+
258+ """Tests share button enable or disabled correctly in multiselection"""
259+ def test_multiselection_share_enabled(self):
260+ self.main_window.swipe_to_gallery(self)
261+ self.move_from_slideshow_to_photogrid()
262+
263+ # Verify options button disabled until we select something
264+ gallery = self.main_window.get_gallery()
265+ opt = gallery.wait_select_single(objectName="additionalActionsButton")
266+ self.assertThat(opt.visible, Eventually(Equals(False)))
267+
268+ # Verify that if we select one photo options and share are enabled
269+ self.select_media(0)
270+ self.assertThat(opt.visible, Eventually(Equals(True)))
271+ self.verify_share_state(True)
272+
273+ # Verify that it stays enabled with mixed media selected
274+ self.select_media(1)
275+ self.verify_share_state(True)
276+
277+ """Tests sharing with mixed media generates a warning dialog"""
278+ def test_no_share_mixed_media(self):
279+ self.main_window.swipe_to_gallery(self)
280+ self.move_from_slideshow_to_photogrid()
281+
282+ self.select_media(0)
283+ self.select_media(1)
284+ share = self.verify_share_state(True, close=False)
285+
286+ self.pointing_device.move_to_object(share)
287+ self.pointing_device.click()
288+
289+ gallery = self.main_window.get_gallery()
290+ gallery.wait_select_single(objectName="unableShareDialog")
291
292=== modified file 'tests/unittests/CMakeLists.txt'
293--- tests/unittests/CMakeLists.txt 2016-02-26 15:24:03 +0000
294+++ tests/unittests/CMakeLists.txt 2016-03-10 12:11:22 +0000
295@@ -7,7 +7,7 @@
296 add_executable(tst_QmlTests tst_QmlTests.cpp)
297 qt5_use_modules(tst_QmlTests Core Qml Quick Test QuickTest)
298 target_link_libraries(tst_QmlTests ${TPL_QT5_LIBRARIES})
299-add_test(tst_QmlTests ${XVFB_RUN_CMD} ${CMAKE_CURRENT_BINARY_DIR}/tst_QmlTests -import ${CMAKE_SOURCE_DIR})
300+add_test(tst_QmlTests ${XVFB_RUN_CMD} ${CMAKE_CURRENT_BINARY_DIR}/tst_QmlTests -import ${CMAKE_SOURCE_DIR} -import ${CMAKE_BINARY_DIR})
301
302 # copy qml test files to build dir
303 file(GLOB qmlTestFiles RELATIVE ${CMAKE_SOURCE_DIR}/tests/unittests/ *qml)
304
305=== added file 'tests/unittests/tst_PhotogridView.qml'
306--- tests/unittests/tst_PhotogridView.qml 1970-01-01 00:00:00 +0000
307+++ tests/unittests/tst_PhotogridView.qml 2016-03-10 12:11:22 +0000
308@@ -0,0 +1,88 @@
309+/*
310+ * Copyright 2016 Canonical Ltd.
311+ *
312+ * This program is free software; you can redistribute it and/or modify
313+ * it under the terms of the GNU General Public License as published by
314+ * the Free Software Foundation; version 3.
315+ *
316+ * This program is distributed in the hope that it will be useful,
317+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
318+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
319+ * GNU General Public License for more details.
320+ *
321+ * You should have received a copy of the GNU General Public License
322+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
323+ *
324+ */
325+
326+import QtQuick 2.4
327+import QtTest 1.0
328+import "../../"
329+import "../../.." //Needed for out of source build
330+
331+TestCase {
332+ name: "PhotogridView"
333+
334+ function test_mixedMediaSelection_data() {
335+ return [
336+ { // one item only
337+ isMixedMedia: false,
338+ listItems: [
339+ { fileType: "image", selected: true, fileURL: "" }
340+ ]
341+ },
342+ { // mixed media but only non-mixed selected
343+ isMixedMedia: false,
344+ listItems: [
345+ { fileType: "video", selected: false, fileURL: "" },
346+ { fileType: "image", selected: true, fileURL: "" },
347+ { fileType: "image", selected: true, fileURL: "" }
348+ ]
349+ },
350+ { // mixed media
351+ isMixedMedia: true,
352+ listItems: [
353+ { fileType: "video", selected: true, fileURL: "" },
354+ { fileType: "image", selected: true, fileURL: "" },
355+ { fileType: "image", selected: true, fileURL: "" }
356+ ]
357+ },
358+ ];
359+ }
360+
361+ function test_mixedMediaSelection(data) {
362+ list.clear()
363+ list.data = data.listItems;
364+ for (var i = 0; i < data.listItems.length; i++) {
365+ list.append(data.listItems[i]);
366+ }
367+ list.updateSelectedFiles();
368+ grid.model = list
369+ compare(grid.selectionContainsMixedMedia(), data.isMixedMedia, "Mixed media not detected correctly")
370+ }
371+
372+ ListModel {
373+ id: list
374+ property var data
375+ property var selectedFiles: []
376+ function updateSelectedFiles() {
377+ // need to re-assign entire list due to the way list properties work in QML
378+ var selected = [];
379+ for (var i = 0; i < list.count; i++) {
380+ if (list.data[i].selected) selected.push(i);
381+ }
382+ list.selectedFiles = selected;
383+ }
384+ function get(i, key) {
385+ return list.data[i][key];
386+ }
387+ }
388+
389+ PhotogridView {
390+ id: grid
391+ width: 600
392+ height: 800
393+ inView: true
394+ inSelectionMode: true
395+ }
396+}

Subscribers

People subscribed via source and target branches