Merge lp:~fboucault/camera-app/camera-app-multi_selection into lp:camera-app
- camera-app-multi_selection
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Bill Filler |
Approved revision: | 429 |
Merged at revision: | 434 |
Proposed branch: | lp:~fboucault/camera-app/camera-app-multi_selection |
Merge into: | lp:camera-app |
Diff against target: |
244 lines (+76/-72) 3 files modified
GalleryView.qml (+7/-68) GalleryViewHeader.qml (+2/-2) PhotogridView.qml (+67/-2) |
To merge this branch: | bzr merge lp:~fboucault/camera-app/camera-app-multi_selection |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Bill Filler (community) | Approve | ||
PS Jenkins bot | continuous-integration | Needs Fixing | |
Florian Boucault | Pending | ||
Review via email: mp+243385@code.launchpad.net |
This proposal supersedes a proposal from 2014-10-31.
Commit message
Add support to multi selection on grid view when triggered by the user
Description of the change
Add support to multi selection on grid view when triggered by the user
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
Merge has conflicts
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
Share and delete dialogs seem to be copy/paste code from elsewhere?
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:413
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:420
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
UNSTABLE: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:420
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
singleSelectionOnly needs to be set to false when activating multi selection by long pressing a photo.
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
What was the point of not having the entire thumbnail area being selectable, ie. tapping anywhere in a photo thumbnail while in multi select mode to add/remove the photo from the selection? I can't remember.
Arthur Mello (artmello) wrote : Posted in a previous version of this proposal | # |
> What was the point of not having the entire thumbnail area being selectable,
> ie. tapping anywhere in a photo thumbnail while in multi select mode to
> add/remove the photo from the selection? I can't remember.
We discussed during the sprint that having the option to view the full photo and not only the thumbnail during a multi selection would be a good feature. But we didn't have a final word if the small check square would be the best way to set a photo as selected.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:422
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
Looks good now, thanks.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:422
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Bill Filler (bfiller) wrote : Posted in a previous version of this proposal | # |
I found a few problems with this MR in testing:
1) Long pressing an image causes the Share/Delete menu to be briefly shown then hidden
2) Select two images, then tap on a third one to view it. When you return to selection mode the previous selections are no longer selected as they should be. Then you can only single select from that point.
3) There should be a "select all" toggle button on the toolbar that toggles select all/select none. See address-book-app. This should perform the same way.
4) I found it's too easy to accidentally tap the photo to open it when I was trying to check the selection button. Maybe leave the visual size of the selection box the same but make a bigger touch area around such that we can increase the hit size without having to make the box bigger visually.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:428
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:433
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Arthur Mello (artmello) wrote : Posted in a previous version of this proposal | # |
All the issues commented by Bill seems to be fixed based on the tests I did on the device. Related with the issue #4 I increased the MouseArea for toggling photo selection to be the full top right quarter of the thumbnail (but did not increased the selection icon itself). Please, let me know if that fix the problem.
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
It looks like it's working fine now. Code is as expected.
Bill Filler (bfiller) wrote : Posted in a previous version of this proposal | # |
works well, but found one small issue. The share action should be disabled if multiple pictures selected as sharing is currently only supported for a single picture in the clients that support it (messaging-app and facebook)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:427
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Bill Filler (bfiller) wrote : | # |
Found another issue:
- have at least 3 pictures in photo roll
- enter selection mode
- select the first two pictures
- click on the third picture which opens it fully
- now perform an action on using the toolbar, either Share or Delete
Expected result:
the current picture which is open should be shared or deleted
Actual result:
the selections from selection view are operated on (shared or deleted)
- 428. By Florian Boucault
-
When in slideshow view and selection mode, make sure that actions correspond to the presently shown photo instead of to the current selection.
- 429. By Florian Boucault
-
Fixed binding loop
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:429
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
UNSTABLE: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
- 430. By Florian Boucault
-
Merged from trunk
Preview Diff
1 | === modified file 'GalleryView.qml' |
2 | --- GalleryView.qml 2014-12-02 12:00:05 +0000 |
3 | +++ GalleryView.qml 2014-12-05 18:44:25 +0000 |
4 | @@ -16,7 +16,6 @@ |
5 | |
6 | import QtQuick 2.2 |
7 | import Ubuntu.Components 1.1 |
8 | -import Ubuntu.Components.Popups 1.0 |
9 | import Ubuntu.Content 0.1 |
10 | import CameraApp 0.1 |
11 | import "MimeTypeMapper.js" as MimeTypeMapper |
12 | @@ -36,26 +35,6 @@ |
13 | singleSelectionOnly: main.transfer.selectionType === ContentTransfer.Single |
14 | } |
15 | |
16 | - property list<Action> userSelectionActions: [ |
17 | - Action { |
18 | - text: i18n.tr("Share") |
19 | - iconName: "share" |
20 | - enabled: model.selectedFiles.length <= 1 |
21 | - onTriggered: { |
22 | - if (model.selectedFiles.length > 0) |
23 | - PopupUtils.open(sharePopoverComponent) |
24 | - } |
25 | - }, |
26 | - Action { |
27 | - text: i18n.tr("Delete") |
28 | - iconName: "delete" |
29 | - onTriggered: { |
30 | - if (model.selectedFiles.length > 0) |
31 | - PopupUtils.open(deleteDialogComponent); |
32 | - } |
33 | - } |
34 | - ] |
35 | - |
36 | property bool gridMode: false |
37 | property bool showLastPhotoTakenPending: false |
38 | |
39 | @@ -97,32 +76,34 @@ |
40 | id: photogridView |
41 | anchors.fill: parent |
42 | headerHeight: header.height |
43 | + userSelectionMode: galleryView.userSelectionMode |
44 | model: galleryView.model |
45 | visible: opacity != 0.0 |
46 | inView: galleryView.inView |
47 | - inSelectionMode: main.contentExportMode || userSelectionMode |
48 | + inSelectionMode: main.contentExportMode || galleryView.userSelectionMode |
49 | onPhotoClicked: { |
50 | slideshowView.showPhotoAtIndex(index); |
51 | galleryView.gridMode = false; |
52 | } |
53 | onPhotoPressAndHold: { |
54 | - if (!userSelectionMode) { |
55 | - userSelectionMode = true; |
56 | + if (!galleryView.userSelectionMode) { |
57 | + galleryView.userSelectionMode = true; |
58 | model.singleSelectionOnly = false; |
59 | model.toggleSelected(index); |
60 | } |
61 | } |
62 | |
63 | onPhotoSelectionAreaClicked: { |
64 | - if (main.contentExportMode || userSelectionMode) |
65 | + if (main.contentExportMode || galleryView.userSelectionMode) |
66 | model.toggleSelected(index); |
67 | } |
68 | + onExitUserSelectionMode: galleryView.exitUserSelectionMode() |
69 | } |
70 | |
71 | // FIXME: it would be better to use the standard header from the toolkit |
72 | GalleryViewHeader { |
73 | id: header |
74 | - actions: userSelectionMode ? userSelectionActions : currentView.actions |
75 | + actions: currentView.actions |
76 | gridMode: galleryView.gridMode || main.contentExportMode |
77 | validationVisible: main.contentExportMode && model.selectedFiles.length > 0 |
78 | userSelectionMode: galleryView.userSelectionMode |
79 | @@ -239,46 +220,4 @@ |
80 | UbuntuNumberAnimation { properties: "scale,opacity"; duration: UbuntuAnimation.SnapDuration } |
81 | } |
82 | ] |
83 | - |
84 | - Component { |
85 | - id: contentItemComp |
86 | - ContentItem {} |
87 | - } |
88 | - |
89 | - Component { |
90 | - id: sharePopoverComponent |
91 | - |
92 | - SharePopover { |
93 | - id: sharePopover |
94 | - |
95 | - onContentPeerSelected: galleryView.exitUserSelectionMode(); |
96 | - |
97 | - transferContentType: MimeTypeMapper.mimeTypeToContentType(model.get(model.selectedFiles[0], "fileType")); |
98 | - transferItems: model.selectedFiles.map(function(row) { |
99 | - return contentItemComp.createObject(parent, {"url": model.get(row, "filePath")}); |
100 | - }) |
101 | - } |
102 | - } |
103 | - |
104 | - Component { |
105 | - id: deleteDialogComponent |
106 | - |
107 | - DeleteDialog { |
108 | - id: deleteDialog |
109 | - |
110 | - FileOperations { |
111 | - id: fileOperations |
112 | - } |
113 | - |
114 | - onDeleteFiles: { |
115 | - for (var i=model.selectedFiles.length-1; i>=0; i--) { |
116 | - var currentFilePath = model.get(model.selectedFiles[i], "filePath"); |
117 | - model.toggleSelected(model.selectedFiles[i]) |
118 | - fileOperations.remove(currentFilePath); |
119 | - } |
120 | - |
121 | - galleryView.exitUserSelectionMode(); |
122 | - } |
123 | - } |
124 | - } |
125 | } |
126 | |
127 | === modified file 'GalleryViewHeader.qml' |
128 | --- GalleryViewHeader.qml 2014-12-02 12:00:05 +0000 |
129 | +++ GalleryViewHeader.qml 2014-12-05 18:44:25 +0000 |
130 | @@ -132,7 +132,7 @@ |
131 | right: parent.right |
132 | } |
133 | width: units.gu(20) |
134 | - height: childrenRect.height |
135 | + height: actionsColumn.height |
136 | clip: actionsColumn.y != 0 |
137 | visible: false |
138 | |
139 | @@ -169,7 +169,7 @@ |
140 | } |
141 | |
142 | Repeater { |
143 | - model: actionsDrawer.actions |
144 | + model: actionsDrawer.actions.length > 0 ? actionsDrawer.actions : 0 |
145 | delegate: AbstractButton { |
146 | id: actionButton |
147 | objectName: "actionButton" + label.text |
148 | |
149 | === modified file 'PhotogridView.qml' |
150 | --- PhotogridView.qml 2014-11-26 15:21:05 +0000 |
151 | +++ PhotogridView.qml 2014-12-05 18:44:25 +0000 |
152 | @@ -15,9 +15,11 @@ |
153 | */ |
154 | |
155 | import QtQuick 2.2 |
156 | -import Ubuntu.Components 1.1 |
157 | +import Ubuntu.Components 1.0 |
158 | +import Ubuntu.Components.Popups 1.0 |
159 | import Ubuntu.Thumbnailer 0.1 |
160 | import Ubuntu.Content 0.1 |
161 | +import CameraApp 0.1 |
162 | import "MimeTypeMapper.js" as MimeTypeMapper |
163 | |
164 | Item { |
165 | @@ -28,10 +30,31 @@ |
166 | signal photoClicked(int index) |
167 | signal photoPressAndHold(int index) |
168 | signal photoSelectionAreaClicked(int index) |
169 | + signal exitUserSelectionMode |
170 | property real headerHeight |
171 | property bool inView |
172 | property bool inSelectionMode |
173 | - property list<Action> actions |
174 | + property bool userSelectionMode: false |
175 | + property var actions: userSelectionMode ? userSelectionActions : [] |
176 | + property list<Action> userSelectionActions: [ |
177 | + Action { |
178 | + text: i18n.tr("Share") |
179 | + iconName: "share" |
180 | + enabled: model.selectedFiles.length <= 1 |
181 | + onTriggered: { |
182 | + if (model.selectedFiles.length > 0) |
183 | + PopupUtils.open(sharePopoverComponent) |
184 | + } |
185 | + }, |
186 | + Action { |
187 | + text: i18n.tr("Delete") |
188 | + iconName: "delete" |
189 | + onTriggered: { |
190 | + if (model.selectedFiles.length > 0) |
191 | + PopupUtils.open(deleteDialogComponent); |
192 | + } |
193 | + } |
194 | + ] |
195 | |
196 | function showPhotoAtIndex(index) { |
197 | gridView.positionViewAtIndex(index, GridView.Center); |
198 | @@ -155,4 +178,46 @@ |
199 | } |
200 | } |
201 | } |
202 | + |
203 | + Component { |
204 | + id: contentItemComp |
205 | + ContentItem {} |
206 | + } |
207 | + |
208 | + Component { |
209 | + id: sharePopoverComponent |
210 | + |
211 | + SharePopover { |
212 | + id: sharePopover |
213 | + |
214 | + onContentPeerSelected: photogridView.exitUserSelectionMode(); |
215 | + |
216 | + transferContentType: MimeTypeMapper.mimeTypeToContentType(model.get(model.selectedFiles[0], "fileType")); |
217 | + transferItems: model.selectedFiles.map(function(row) { |
218 | + return contentItemComp.createObject(parent, {"url": model.get(row, "filePath")}); |
219 | + }) |
220 | + } |
221 | + } |
222 | + |
223 | + Component { |
224 | + id: deleteDialogComponent |
225 | + |
226 | + DeleteDialog { |
227 | + id: deleteDialog |
228 | + |
229 | + FileOperations { |
230 | + id: fileOperations |
231 | + } |
232 | + |
233 | + onDeleteFiles: { |
234 | + for (var i=model.selectedFiles.length-1; i>=0; i--) { |
235 | + var currentFilePath = model.get(model.selectedFiles[i], "filePath"); |
236 | + model.toggleSelected(model.selectedFiles[i]) |
237 | + fileOperations.remove(currentFilePath); |
238 | + } |
239 | + |
240 | + photogridView.exitUserSelectionMode(); |
241 | + } |
242 | + } |
243 | + } |
244 | } |
FAILED: Continuous integration, rev:412 jenkins. qa.ubuntu. com/job/ camera- app-ci/ 301/ jenkins. qa.ubuntu. com/job/ camera- app-utopic- amd64-ci/ 117/console jenkins. qa.ubuntu. com/job/ camera- app-utopic- armhf-ci/ 117/console jenkins. qa.ubuntu. com/job/ camera- app-utopic- i386-ci/ 117/console jenkins. qa.ubuntu. com/job/ generic- click-autopilot -utopic- touch/496/ console jenkins. qa.ubuntu. com/job/ generic- mediumtests- utopic/ 4254/console jenkins. qa.ubuntu. com/job/ generic- click-builder- utopic- armhf/860/ console jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- utopic- amd64/4646/ console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/camera- app-ci/ 301/rebuild
http://