Merge lp:~gang65/ubuntu-calculator-app/reboot_copy_selected_calculation into lp:ubuntu-calculator-app

Proposed by Bartosz Kosiorek
Status: Merged
Approved by: Giulio Collura
Approved revision: 65
Merged at revision: 66
Proposed branch: lp:~gang65/ubuntu-calculator-app/reboot_copy_selected_calculation
Merge into: lp:ubuntu-calculator-app
Diff against target: 101 lines (+40/-7)
1 file modified
app/ubuntu-calculator-app.qml (+40/-7)
To merge this branch: bzr merge lp:~gang65/ubuntu-calculator-app/reboot_copy_selected_calculation
Reviewer Review Type Date Requested Status
Giulio Collura Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Riccardo Padovani Pending
Review via email: mp+246505@code.launchpad.net

Commit message

Add feature to copy selected calculation from the history

Description of the change

Add feature to copy selected calculation from the history

To post a comment you must log in.
Revision history for this message
Giulio Collura (gcollura) wrote :

We need to set action names for our actions, otherwise we end up with a empty label in overflow action menu, see http://i.imgur.com/oqc9gSQ.png

Thanks!

Giulio

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Thanks Giulio.

I was wondering why we need these additional text caption, while we are using icons :-P

63. By Bartosz Kosiorek

Add captions to action buttons

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Giulio Collura (gcollura) wrote :

Do we really need to copy the date in the clipboard?
I copied a couple of calculation as test and the date just stand in the way without a real meaning, see:
[Wed Jan 14 23:20:19 2015 GMT+0100] 2+6=8
[Wed Jan 14 23:20:19 2015 GMT+0100] -10^2/5=-20
The actual calculation is short, while the date and time takes a lot of space. I'd like also to discuss about this with rpadovani :)

Also, we may want to support the "copy to clipboard" for a single entry, be swiping the list item to the left, similarly to delete, which is shown with a swipe to right gesture.

I also left a comment inline.

review: Needs Information
Revision history for this message
Riccardo Padovani (rpadovani) wrote :

I agree with Giulio, I think we don't need the date in the clipboard or, if we want it, we definitely need to change the format

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Ok. I will remove date.

64. By Bartosz Kosiorek

Remove date from copied data

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

As you already noticed, copied data is in engine format, without formatting.
In my opinion is simpler to modify ASCII data, rather than UTF-8 chars.

For example:

1/0=Infinity

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

As I remember, according to design swipe to left was reserved for adding calculation to favourites.

Favourites is not implemented yet. In my opinion copy has higher priority.

Revision history for this message
Giulio Collura (gcollura) wrote :

We can have two rightSideActions in ListItemWithActions, one action for copy another for favouring, because it can be tedious to start the selection mode, just to copy a single calculation. Anyway if this a design pattern and since I'm not a designer, I can't judge :)

I almost forgot, we have to filter out the entries in the model which have been hidden and have a dbId < 0, otherwise a empty calculation will be copied, if these get selected when selecting all the entries (I know this may be a bug in MultipleSelectionVisualModel, but it's better to workaround it here in this case). Just put a 'if (items.get(j).model.dbId !== -1)' before adding the calculations to mimeData.text.

Thank you!

Giulio

review: Needs Fixing
65. By Bartosz Kosiorek

Do not copy calculation with dbId === -1

Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Nice catch Giulio.

Please also review other Merge Reqest.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Giulio Collura (gcollura) wrote :

Ok thank you for the patience and the merge proposal :)
Approved!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'app/ubuntu-calculator-app.qml'
--- app/ubuntu-calculator-app.qml 2015-01-14 14:47:03 +0000
+++ app/ubuntu-calculator-app.qml 2015-01-15 18:57:16 +0000
@@ -62,6 +62,9 @@
6262
63 property var decimalPoint: Qt.locale().decimalPoint63 property var decimalPoint: Qt.locale().decimalPoint
6464
65 // By default we delete selected calculation from history
66 property bool deleteSelectedCalculation: true;
67
65 state: visualModel.isInSelectionMode ? "selection" : "default"68 state: visualModel.isInSelectionMode ? "selection" : "default"
66 states: [69 states: [
67 State {70 State {
@@ -227,16 +230,25 @@
227 }230 }
228 actions: [231 actions: [
229 Action {232 Action {
233 id: copySelectedAction
234 objectName: "copySelectedAction"
235 iconName: "edit-copy"
236 text: i18n.tr("Copy")
237 onTriggered: copySelectedCalculations()
238 },
239 Action {
230 id: selectAllAction240 id: selectAllAction
231 objectName: "selectAllAction"241 objectName: "selectAllAction"
232 iconName: "select"242 iconName: "select"
243 text: i18n.tr("Select All")
233 onTriggered: visualModel.selectAll()244 onTriggered: visualModel.selectAll()
234 },245 },
235 Action {246 Action {
236 id: multiDeleteAction247 id: multiDeleteAction
237 objectName: "multiDeleteAction"248 objectName: "multiDeleteAction"
238 iconName: "delete"249 iconName: "delete"
239 onTriggered: visualModel.endSelection()250 text: i18n.tr("Delete")
251 onTriggered: deleteSelectedCalculations()
240 }252 }
241 ]253 ]
242 }254 }
@@ -254,7 +266,7 @@
254 width: parent ? parent.width : 0266 width: parent ? parent.width : 0
255267
256 property var model: itemModel268 property var model: itemModel
257 visible: model.dbId != -1269 visible: model.dbId !== -1
258270
259 selectionMode: visualModel.isInSelectionMode271 selectionMode: visualModel.isInSelectionMode
260 selected: visualModel.isSelected(visualDelegate)272 selected: visualModel.isSelected(visualDelegate)
@@ -327,13 +339,34 @@
327 }339 }
328 }340 }
329341
342 function deleteSelectedCalculations() {
343 deleteSelectedCalculation = true;
344 visualModel.endSelection();
345 }
346
347 function copySelectedCalculations() {
348 deleteSelectedCalculation = false;
349 visualModel.endSelection();
350 }
351
330 MultipleSelectionVisualModel {352 MultipleSelectionVisualModel {
331 id: visualModel353 id: visualModel
332 model: calculationHistory.getContents()354 model: calculationHistory.getContents()
333355
334 onSelectionDone: {356 onSelectionDone: {
335 for (var i = 0; i < items.count; i++) {357 if(deleteSelectedCalculation === true) {
336 calculationHistory.deleteCalc(items.get(i).model.dbId, items.get(i).model.index);358 for(var i = 0; i < items.count; i++) {
359 calculationHistory.deleteCalc(items.get(i).model.dbId, items.get(i).model.index);
360 }
361 } else {
362 var mimeData = Clipboard.newData();
363 mimeData.text = "";
364 for(var j = 0; j < items.count; j++) {
365 if (items.get(j).model.dbId !== -1) {
366 mimeData.text = mimeData.text + items.get(j).model.formula + "=" + items.get(j).model.result + "\n";
367 }
368 }
369 Clipboard.push(mimeData);
337 }370 }
338 }371 }
339372
@@ -341,11 +374,11 @@
341 Loader {374 Loader {
342 property var itemModel: model375 property var itemModel: model
343 width: parent.width376 width: parent.width
344 height: model.dbId != -1 ? item.height : 0;377 height: model.dbId !== -1 ? item.height : 0;
345 sourceComponent: screenDelegateComponent378 sourceComponent: screenDelegateComponent
346 opacity: ((y+height) >= scrollableView.contentY) && (y <= (scrollableView.contentY + scrollableView.height)) ? 1 : 0379 opacity: ((y + height) >= scrollableView.contentY) && (y <= (scrollableView.contentY + scrollableView.height)) ? 1 : 0
347 onOpacityChanged: {380 onOpacityChanged: {
348 if (this.hasOwnProperty('item') && this.item != null) {381 if (this.hasOwnProperty('item') && this.item !== null) {
349 if (opacity > 0) {382 if (opacity > 0) {
350 sourceComponent = screenDelegateComponent;383 sourceComponent = screenDelegateComponent;
351 } else {384 } else {

Subscribers

People subscribed via source and target branches