Merge lp:~uriboni/camera-app/share-multiple-items into lp:camera-app/staging
- share-multiple-items
- Merge into staging
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 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot | continuous-integration | Needs Fixing | |
Florian Boucault (community) | Needs Fixing | ||
Review via email:
|
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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Florian Boucault (fboucault) wrote : | # |
In some cases when selecting 3 photos camera thinks I have mixed type contents. The function selectedFilesMi
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:662
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:663
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:665
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 666. By Ugo Riboni
-
Allow unit tests to find c++ plugin when run out of tree
- 667. By Ugo Riboni
-
Fix PEP8 issue
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:667
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 668. By Ugo Riboni
-
Add build dependencies needed for unit tests
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:668
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 669. By Ugo Riboni
-
Merge parent branch
- 670. By Ugo Riboni
-
Fix flake8 errors
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:668
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Preview Diff
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 | +} |
code in PhotogridView. qml/onTriggered could be written in a clearer way, for example:
onTriggered: { ixedTypes( )) { open(unableShar eDialogComponen t).parent = photogridView; open(sharePopov erComponent) .parent = photogridView;
if (selectedFilesM
PopupUtils.
} else {
PopupUtils.
}
}