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
=== modified file 'GalleryView.qml'
--- GalleryView.qml 2016-02-23 11:46:52 +0000
+++ GalleryView.qml 2016-03-10 12:11:22 +0000
@@ -35,7 +35,7 @@
35 StorageLocations.removableStorageVideosLocation]35 StorageLocations.removableStorageVideosLocation]
36 typeFilters: !main.contentExportMode ? [ "image", "video" ]36 typeFilters: !main.contentExportMode ? [ "image", "video" ]
37 : [MimeTypeMapper.contentTypeToMimeType(main.transferContentType)]37 : [MimeTypeMapper.contentTypeToMimeType(main.transferContentType)]
38 singleSelectionOnly: main.transfer.selectionType === ContentTransfer.Single38 singleSelectionOnly: main.contentExportMode && main.transfer.selectionType === ContentTransfer.Single
39 }39 }
4040
41 property bool gridMode: main.contentExportMode41 property bool gridMode: main.contentExportMode
@@ -58,7 +58,6 @@
5858
59 function exitUserSelectionMode() {59 function exitUserSelectionMode() {
60 model.clearSelection();60 model.clearSelection();
61 model.singleSelectionOnly = true;
62 userSelectionMode = false;61 userSelectionMode = false;
63 }62 }
6463
@@ -97,7 +96,6 @@
97 onPhotoPressAndHold: {96 onPhotoPressAndHold: {
98 if (!galleryView.userSelectionMode) {97 if (!galleryView.userSelectionMode) {
99 galleryView.userSelectionMode = true;98 galleryView.userSelectionMode = true;
100 model.singleSelectionOnly = false;
101 model.toggleSelected(index);99 model.toggleSelected(index);
102 }100 }
103 }101 }
104102
=== modified file 'PhotogridView.qml'
--- PhotogridView.qml 2016-03-03 14:12:26 +0000
+++ PhotogridView.qml 2016-03-10 12:11:22 +0000
@@ -40,11 +40,14 @@
40 Action {40 Action {
41 text: i18n.tr("Share")41 text: i18n.tr("Share")
42 iconName: "share"42 iconName: "share"
43 enabled: model.selectedFiles.length <= 143 enabled: model.selectedFiles.length > 0
44 onTriggered: {44 onTriggered: {
45 if (model.selectedFiles.length > 0) {45 // Display a warning message if we are attempting to share mixed
46 var dialog = PopupUtils.open(sharePopoverComponent)46 // content, as the framework does not properly support this
47 dialog.parent = photogridView47 if (selectionContainsMixedMedia()) {
48 PopupUtils.open(unableShareDialogComponent).parent = photogridView;
49 } else {
50 PopupUtils.open(sharePopoverComponent).parent = photogridView;
48 }51 }
49 }52 }
50 },53 },
@@ -60,6 +63,19 @@
60 }63 }
61 ]64 ]
6265
66 function selectionContainsMixedMedia() {
67 var selection = model.selectedFiles;
68 var lastType = model.get(selection[0], "fileType");
69 for (var i = 1; i < selection.length; i++) {
70 var type = model.get(selection[i], "fileType");
71 if (type !== lastType) {
72 return true;
73 }
74 lastType = type;
75 }
76 return false;
77 }
78
63 function showPhotoAtIndex(index) {79 function showPhotoAtIndex(index) {
64 gridView.positionViewAtIndex(index, GridView.Center);80 gridView.positionViewAtIndex(index, GridView.Center);
65 }81 }
@@ -170,6 +186,7 @@
170 visible: inSelectionMode186 visible: inSelectionMode
171187
172 Icon {188 Icon {
189 objectName: "mediaItemCheckBox"
173 anchors.centerIn: parent190 anchors.centerIn: parent
174 width: parent.width * 0.8191 width: parent.width * 0.8
175 height: parent.height * 0.8192 height: parent.height * 0.8
@@ -235,4 +252,13 @@
235 onVisibleChanged: photogridView.toggleHeader()252 onVisibleChanged: photogridView.toggleHeader()
236 }253 }
237 }254 }
255
256 Component {
257 id: unableShareDialogComponent
258 UnableShareDialog {
259 objectName: "unableShareDialog"
260 onVisibleChanged: photogridView.toggleHeader()
261 }
262 }
263
238}264}
239265
=== added file 'UnableShareDialog.qml'
--- UnableShareDialog.qml 1970-01-01 00:00:00 +0000
+++ UnableShareDialog.qml 2016-03-10 12:11:22 +0000
@@ -0,0 +1,34 @@
1/*
2 * Copyright (C) 2016 Canonical Ltd
3 *
4 * This program is free software: you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License version 3 as
6 * published by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.4
18import Ubuntu.Components 1.3
19import Ubuntu.Components.Popups 1.3
20
21Dialog {
22 id: dialog
23 objectName: "unableShareDialog"
24
25 title: i18n.tr("Unable to share")
26 text: i18n.tr("Unable to share photos and videos at the same time")
27
28 Button {
29 objectName: "unableShareDialogOk"
30 text: i18n.tr("Ok")
31 color: UbuntuColors.orange
32 onClicked: PopupUtils.close(dialog);
33 }
34}
035
=== modified file 'camera-app.qml'
--- camera-app.qml 2016-03-08 14:42:51 +0000
+++ camera-app.qml 2016-03-10 12:11:22 +0000
@@ -282,7 +282,7 @@
282282
283 property bool contentExportMode: transfer !== null283 property bool contentExportMode: transfer !== null
284 property var transfer: null284 property var transfer: null
285 property var transferContentType: ContentType.Pictures285 property var transferContentType: transfer ? transfer.contentType : "image"
286286
287 function exportContent(urls) {287 function exportContent(urls) {
288 if (!main.transfer) return;288 if (!main.transfer) return;
289289
=== modified file 'debian/control'
--- debian/control 2015-07-31 02:23:54 +0000
+++ debian/control 2016-03-10 12:11:22 +0000
@@ -13,11 +13,14 @@
13 qtbase5-dev,13 qtbase5-dev,
14 qtdeclarative5-dev,14 qtdeclarative5-dev,
15 qml-module-qtquick2,15 qml-module-qtquick2,
16 qml-module-qtpositioning,
16 qml-module-qttest,17 qml-module-qttest,
17 qtdeclarative5-ubuntu-ui-toolkit-plugin,18 qtdeclarative5-ubuntu-ui-toolkit-plugin,
18 qtdeclarative5-unity-action-plugin (>= 1.1.0),19 qtdeclarative5-unity-action-plugin (>= 1.1.0),
19 qtdeclarative5-usermetrics0.1,20 qtdeclarative5-usermetrics0.1,
20 qtdeclarative5-ubuntu-content1,21 qtdeclarative5-ubuntu-content1,
22 qtdeclarative5-ubuntu-thumbnailer0.1,
23 qtdeclarative5-ubuntu-ui-extras0.2,
21 qtmultimedia5-dev,24 qtmultimedia5-dev,
22 libusermetricsinput1-dev,25 libusermetricsinput1-dev,
23 gettext,26 gettext,
2427
=== modified file 'tests/autopilot/camera_app/tests/test_gallery_view.py'
--- tests/autopilot/camera_app/tests/test_gallery_view.py 2016-03-02 14:35:50 +0000
+++ tests/autopilot/camera_app/tests/test_gallery_view.py 2016-03-10 12:11:22 +0000
@@ -35,16 +35,21 @@
35 self.assertThat(slideshow_view.visible, Eventually(Equals(False)))35 self.assertThat(slideshow_view.visible, Eventually(Equals(False)))
36 self.assertThat(photogrid_view.visible, Eventually(Equals(True)))36 self.assertThat(photogrid_view.visible, Eventually(Equals(True)))
3737
38 def select_first_photo(self):38 def select_media(self, index=0):
39 # select the first photo39 # select the photo with index, default to the first one
40 gallery = self.main_window.get_gallery()40 gallery = self.main_window.get_gallery()
41 photo = gallery.wait_select_single(objectName="mediaItem0")41 photo = gallery.wait_select_single(objectName="mediaItem" + str(index))
42 self.pointing_device.move_to_object(photo)42 checkbox = photo.wait_select_single(objectName="mediaItemCheckBox")
4343
44 # do a long press to enter Multiselection mode44 self.pointing_device.move_to_object(checkbox)
45 self.pointing_device.press()45
46 sleep(1)46 if checkbox.visible:
47 self.pointing_device.release()47 self.click()
48 else:
49 # do a long press to enter Multiselection mode
50 self.pointing_device.press()
51 sleep(1)
52 self.pointing_device.release()
4853
4954
50class TestCameraGalleryView(CameraAppTestCase, TestCameraGalleryViewMixin):55class TestCameraGalleryView(CameraAppTestCase, TestCameraGalleryViewMixin):
@@ -186,7 +191,7 @@
186 def test_delete_photo_from_multiselection(self):191 def test_delete_photo_from_multiselection(self):
187 self.main_window.swipe_to_gallery(self)192 self.main_window.swipe_to_gallery(self)
188 self.move_from_slideshow_to_photogrid()193 self.move_from_slideshow_to_photogrid()
189 self.select_first_photo()194 self.select_media()
190195
191 # open actions drawer196 # open actions drawer
192 gallery = self.main_window.get_gallery()197 gallery = self.main_window.get_gallery()
@@ -211,7 +216,7 @@
211 def test_multiselection_mode(self):216 def test_multiselection_mode(self):
212 self.main_window.swipe_to_gallery(self)217 self.main_window.swipe_to_gallery(self)
213 self.move_from_slideshow_to_photogrid()218 self.move_from_slideshow_to_photogrid()
214 self.select_first_photo()219 self.select_media()
215220
216 # exit the multiselection mode221 # exit the multiselection mode
217 gallery = self.main_window.get_gallery()222 gallery = self.main_window.get_gallery()
@@ -224,3 +229,72 @@
224229
225 self.assertThat(slideshow_view.visible, Eventually(Equals(False)))230 self.assertThat(slideshow_view.visible, Eventually(Equals(False)))
226 self.assertThat(photogrid_view.visible, Eventually(Equals(True)))231 self.assertThat(photogrid_view.visible, Eventually(Equals(True)))
232
233
234class TestCameraGalleryViewWithPhotosAndVideo(
235 TestCameraGalleryViewMixin, CameraAppTestCase):
236 """Tests the camera gallery view with two photos and a video"""
237
238 def setUp(self):
239 self.delete_all_media()
240 self.add_sample_photo()
241 self.add_sample_video()
242
243 super(TestCameraGalleryViewWithPhotosAndVideo, self).setUp()
244 self.assertThat(
245 self.main_window.get_qml_view().visible, Eventually(Equals(True)))
246
247 def tearDown(self):
248 super(TestCameraGalleryViewWithPhotosAndVideo, self).tearDown()
249
250 def verify_share_state(self, expectedState, close=True):
251 # open actions drawer
252 gallery = self.main_window.get_gallery()
253 opt = gallery.wait_select_single(objectName="additionalActionsButton")
254 self.pointing_device.move_to_object(opt)
255 self.pointing_device.click()
256
257 # verify expected state
258 share = gallery.wait_select_single(objectName="actionButtonShare")
259 self.assertThat(share.enabled, Eventually(Equals(expectedState)))
260
261 if (close):
262 # close actions drawer
263 self.pointing_device.move_to_object(opt)
264 self.pointing_device.click()
265 else:
266 return share
267
268 """Tests share button enable or disabled correctly in multiselection"""
269 def test_multiselection_share_enabled(self):
270 self.main_window.swipe_to_gallery(self)
271 self.move_from_slideshow_to_photogrid()
272
273 # Verify options button disabled until we select something
274 gallery = self.main_window.get_gallery()
275 opt = gallery.wait_select_single(objectName="additionalActionsButton")
276 self.assertThat(opt.visible, Eventually(Equals(False)))
277
278 # Verify that if we select one photo options and share are enabled
279 self.select_media(0)
280 self.assertThat(opt.visible, Eventually(Equals(True)))
281 self.verify_share_state(True)
282
283 # Verify that it stays enabled with mixed media selected
284 self.select_media(1)
285 self.verify_share_state(True)
286
287 """Tests sharing with mixed media generates a warning dialog"""
288 def test_no_share_mixed_media(self):
289 self.main_window.swipe_to_gallery(self)
290 self.move_from_slideshow_to_photogrid()
291
292 self.select_media(0)
293 self.select_media(1)
294 share = self.verify_share_state(True, close=False)
295
296 self.pointing_device.move_to_object(share)
297 self.pointing_device.click()
298
299 gallery = self.main_window.get_gallery()
300 gallery.wait_select_single(objectName="unableShareDialog")
227301
=== modified file 'tests/unittests/CMakeLists.txt'
--- tests/unittests/CMakeLists.txt 2016-02-26 15:24:03 +0000
+++ tests/unittests/CMakeLists.txt 2016-03-10 12:11:22 +0000
@@ -7,7 +7,7 @@
7add_executable(tst_QmlTests tst_QmlTests.cpp)7add_executable(tst_QmlTests tst_QmlTests.cpp)
8qt5_use_modules(tst_QmlTests Core Qml Quick Test QuickTest)8qt5_use_modules(tst_QmlTests Core Qml Quick Test QuickTest)
9target_link_libraries(tst_QmlTests ${TPL_QT5_LIBRARIES})9target_link_libraries(tst_QmlTests ${TPL_QT5_LIBRARIES})
10add_test(tst_QmlTests ${XVFB_RUN_CMD} ${CMAKE_CURRENT_BINARY_DIR}/tst_QmlTests -import ${CMAKE_SOURCE_DIR})10add_test(tst_QmlTests ${XVFB_RUN_CMD} ${CMAKE_CURRENT_BINARY_DIR}/tst_QmlTests -import ${CMAKE_SOURCE_DIR} -import ${CMAKE_BINARY_DIR})
1111
12# copy qml test files to build dir12# copy qml test files to build dir
13file(GLOB qmlTestFiles RELATIVE ${CMAKE_SOURCE_DIR}/tests/unittests/ *qml)13file(GLOB qmlTestFiles RELATIVE ${CMAKE_SOURCE_DIR}/tests/unittests/ *qml)
1414
=== added file 'tests/unittests/tst_PhotogridView.qml'
--- tests/unittests/tst_PhotogridView.qml 1970-01-01 00:00:00 +0000
+++ tests/unittests/tst_PhotogridView.qml 2016-03-10 12:11:22 +0000
@@ -0,0 +1,88 @@
1/*
2 * Copyright 2016 Canonical Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 *
16 */
17
18import QtQuick 2.4
19import QtTest 1.0
20import "../../"
21import "../../.." //Needed for out of source build
22
23TestCase {
24 name: "PhotogridView"
25
26 function test_mixedMediaSelection_data() {
27 return [
28 { // one item only
29 isMixedMedia: false,
30 listItems: [
31 { fileType: "image", selected: true, fileURL: "" }
32 ]
33 },
34 { // mixed media but only non-mixed selected
35 isMixedMedia: false,
36 listItems: [
37 { fileType: "video", selected: false, fileURL: "" },
38 { fileType: "image", selected: true, fileURL: "" },
39 { fileType: "image", selected: true, fileURL: "" }
40 ]
41 },
42 { // mixed media
43 isMixedMedia: true,
44 listItems: [
45 { fileType: "video", selected: true, fileURL: "" },
46 { fileType: "image", selected: true, fileURL: "" },
47 { fileType: "image", selected: true, fileURL: "" }
48 ]
49 },
50 ];
51 }
52
53 function test_mixedMediaSelection(data) {
54 list.clear()
55 list.data = data.listItems;
56 for (var i = 0; i < data.listItems.length; i++) {
57 list.append(data.listItems[i]);
58 }
59 list.updateSelectedFiles();
60 grid.model = list
61 compare(grid.selectionContainsMixedMedia(), data.isMixedMedia, "Mixed media not detected correctly")
62 }
63
64 ListModel {
65 id: list
66 property var data
67 property var selectedFiles: []
68 function updateSelectedFiles() {
69 // need to re-assign entire list due to the way list properties work in QML
70 var selected = [];
71 for (var i = 0; i < list.count; i++) {
72 if (list.data[i].selected) selected.push(i);
73 }
74 list.selectedFiles = selected;
75 }
76 function get(i, key) {
77 return list.data[i][key];
78 }
79 }
80
81 PhotogridView {
82 id: grid
83 width: 600
84 height: 800
85 inView: true
86 inSelectionMode: true
87 }
88}

Subscribers

People subscribed via source and target branches