Merge lp:~ubuntu-calculator-dev/ubuntu-calculator-app/ubuntu-calculator-app-remember-last-formula into lp:ubuntu-calculator-app

[Bartosz Kosiorek]

Remember formula from InputField after application close (LP: #1520508)

Remember formula from InputField after application close (LP: #1520508)

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:247
https://core-apps-jenkins.ubuntu.com/job/calculator-app-ci/35/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/200/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/calculator-app-ci/35/rebuild

review: Approve (continuous-integration)

[Riccardo Padovani (rpadovani)]

Hi Bartosz,
thanks for working on this.

Unfortunately, I don't think this is the right way to fix the problem: Qt.labs.settings writes on a file the setting, so you add a very big overhead: every time user presses a button there is an I/O operation, that is very expensive.

IMO the right direction to fix this bug is saving the actual formula only when app is closed (using Component.onDestruction) and when it losts focus (creating a bind on Qt.application.state[0]).

Let me know what you think.

[0]: http://doc.qt.io/qt-5/qml-qtqml-qt.html#application-prop

[Bartosz Kosiorek (gang65)]

From user point of view it will be great to save progress every change.

I will check the impact on performance and give you performance comparison.

[Bartosz Kosiorek (gang65)]

I checked that and saving is very fast, so from user perspective there is more benefits of savings formula every change (due to out of memory, which happens when you will open too many tabs in webbrowser).

Could you please double check that issue with profiler?

review: Needs Information

[Bartosz Kosiorek (gang65)]

I checked that and saving is very fast, so from user perspective there is more benefits of savings formula every change (due to out of memory, which happens when you will open too many tabs in webbrowser).

Could you please double check that issue with profiler?

review: Needs Information

Unmerged revisions

247. By Bartosz Kosiorek on 2015-11-28

Remember formula from InputField after application close (LP: #1520508)

=== modified file 'app/ubuntu-calculator-app.qml'
--- app/ubuntu-calculator-app.qml	2015-11-28 20:44:02 +0000
+++ app/ubuntu-calculator-app.qml	2015-11-28 23:18:30 +0000
@@ -24,6 +24,7 @@
 import "engine"
 import "engine/math.js" as MathJs
 import "engine/formula.js" as Formula
+import Qt.labs.settings 1.0
 
 MainView {
     id: mainView
@@ -40,14 +41,16 @@
     // This is our engine
     property var mathJs: MathJs.mathJs;
 
-    // Long form of formula, which are saved in the storage/history
-    property string longFormula: "";
-
-    // Engine's short form of formula. It is displayed in TextInput field
-    property string shortFormula: "";
-
-    // The formula converted to human eye, which will be displayed in text input field
-    property string displayedInputText: "";
+    property var settings: Settings {
+      // Long form of formula, which are saved in the storage/history
+      property string longFormula: "";
+
+      // Engine's short form of formula. It is displayed in TextInput field
+      property string shortFormula: "";
+
+      // The formula converted to human eye, which will be displayed in text input field
+      property string displayedInputText: "";
+    }
 
     // If this is true we calculate a temporary result to show in the bottom label
     property bool isFormulaIsValidToCalculate: false;
@@ -77,14 +80,14 @@
     function deleteLastFormulaElement() {
         isFormulaIsValidToCalculate = false;
         if (textInputField.cursorPosition === textInputField.length) {
-            longFormula = Formula.deleteLastFormulaElement(isLastCalculate, longFormula)
+            settings.longFormula = Formula.deleteLastFormulaElement(isLastCalculate, settings.longFormula)
         } else {
-            var truncatedSubstring = Formula.deleteLastFormulaElement(isLastCalculate, longFormula.slice(0, textInputField.cursorPosition))
-            longFormula = truncatedSubstring + longFormula.slice(textInputField.cursorPosition, longFormula.length);
+            var truncatedSubstring = Formula.deleteLastFormulaElement(isLastCalculate, settings.longFormula.slice(0, textInputField.cursorPosition))
+            settings.longFormula = truncatedSubstring + settings.longFormula.slice(textInputField.cursorPosition, settings.longFormula.length);
         }
-        shortFormula = longFormula;
+        settings.shortFormula = settings.longFormula;
 
-        displayedInputText = longFormula;
+        settings.displayedInputText = settings.longFormula;
         if (truncatedSubstring) {
             textInputField.cursorPosition = truncatedSubstring.length;
         }
@@ -95,9 +98,9 @@
      */
     function clearFormula() {
         isFormulaIsValidToCalculate = false;
-        shortFormula = "";
-        longFormula = "";
-        displayedInputText = "";
+        settings.shortFormula = "";
+        settings.longFormula = "";
+        settings.displayedInputText = "";
     }
 
     /**
@@ -126,28 +129,28 @@
         // formula, otherwise we continue with the old one
         if ((!isNaN(visual) || (visual === ".")) && isLastCalculate) {
             isFormulaIsValidToCalculate = false;
-            longFormula = displayedInputText = shortFormula = "";
+            settings.longFormula = settings.displayedInputText = settings.shortFormula = "";
         }
         // Add zero when decimal separator is not after number
-        if ((visual === ".") && ((isNaN(displayedInputText.slice(textInputField.cursorPosition - 1, textInputField.cursorPosition))) || (longFormula === ""))) {
+        if ((visual === ".") && ((isNaN(settings.displayedInputText.slice(textInputField.cursorPosition - 1, textInputField.cursorPosition))) || (settings.longFormula === ""))) {
             visual = "0.";
         }
         isLastCalculate = false;
 
-        // Validate whole longFormula if the cursor is at the end of string
+        // Validate whole settings.longFormula if the cursor is at the end of string
         if (textInputField.cursorPosition === textInputField.length) {
             if (visual === "()") {
-                visual = Formula.determineBracketTypeToAdd(longFormula)
+                visual = Formula.determineBracketTypeToAdd(settings.longFormula)
             }
-            if (Formula.validateStringForAddingToFormula(longFormula, visual) === false) {
+            if (Formula.validateStringForAddingToFormula(settings.longFormula, visual) === false) {
                 errorAnimation.restart();
                 return;
             }
         } else {
             if (visual === "()") {
-                visual = Formula.determineBracketTypeToAdd(longFormula.slice(0, textInputField.cursorPosition))
+                visual = Formula.determineBracketTypeToAdd(settings.longFormula.slice(0, textInputField.cursorPosition))
             }
-            if (Formula.validateStringForAddingToFormula(longFormula.slice(0, textInputField.cursorPosition), visual) === false) {
+            if (Formula.validateStringForAddingToFormula(settings.longFormula.slice(0, textInputField.cursorPosition), visual) === false) {
                 errorAnimation.restart();
                 return;
             }
@@ -160,9 +163,9 @@
         // we display a temporary result instead the all operation
         if (isNaN(visual) && (visual.toString() !== ".") && isFormulaIsValidToCalculate) {
             try {
-                shortFormula = formatBigNumber(mathJs.eval(shortFormula));
+                settings.shortFormula = formatBigNumber(mathJs.eval(settings.shortFormula));
             } catch(exception) {
-                console.log("Debug: Temp result: " + exception.toString() + " engine formula: " + shortFormula);
+                console.log("Debug: Temp result: " + exception.toString() + " engine formula: " + settings.shortFormula);
             }
 
             isFormulaIsValidToCalculate = false;
@@ -170,14 +173,14 @@
 
         // Adding the new operator to the formula
         if (textInputField.cursorPosition === textInputField.length ) {
-            longFormula += visual.toString();
-            shortFormula += visual.toString();
-            displayedInputText = shortFormula;
+            settings.longFormula += visual.toString();
+            settings.shortFormula += visual.toString();
+            settings.displayedInputText = settings.shortFormula;
         } else {
-            longFormula = longFormula.slice(0, textInputField.cursorPosition) + visual.toString() + longFormula.slice(textInputField.cursorPosition, longFormula.length);
-            shortFormula = longFormula;
+            settings.longFormula = settings.longFormula.slice(0, textInputField.cursorPosition) + visual.toString() + settings.longFormula.slice(textInputField.cursorPosition, settings.longFormula.length);
+            settings.shortFormula = settings.longFormula;
             var preservedCursorPosition = textInputField.cursorPosition;
-            displayedInputText = shortFormula;
+            settings.displayedInputText = settings.shortFormula;
             textInputField.cursorPosition = preservedCursorPosition + visual.length;
         }
 
@@ -191,21 +194,21 @@
         mathJs.config({
                 number: 'bignumber'
         });
-        if ((longFormula === '') || (isLastCalculate === true)) {
+        if ((settings.longFormula === '') || (isLastCalculate === true)) {
             errorAnimation.restart();
             return;
         }
 
         // We try to balance brackets to avoid mathJs errors
-        var numberOfOpenedBrackets = (longFormula.match(/\(/g) || []).length -
-                                        (longFormula.match(/\)/g) || []).length;
+        var numberOfOpenedBrackets = (settings.longFormula.match(/\(/g) || []).length -
+                                        (settings.longFormula.match(/\)/g) || []).length;
 
         for (var i = 0; i < numberOfOpenedBrackets; i++) {
             formulaPush(')');
         }
 
         try {
-            var result = mathJs.eval(longFormula);
+            var result = mathJs.eval(settings.longFormula);
 
             result = formatBigNumber(result)
 
@@ -220,17 +223,17 @@
 
         isLastCalculate = true;
 
-        if (result === longFormula) {
+        if (result === settings.longFormula) {
             errorAnimation.restart();
             return;
         }
 
-        calculationHistory.addCalculationToScreen(longFormula, result, false, "");
+        calculationHistory.addCalculationToScreen(settings.longFormula, result, false, "");
         editedCalculationIndex = -1;
-        longFormula = result;
-        shortFormula = result;
+        settings.longFormula = result;
+        settings.shortFormula = result;
         favouriteTextField.text = "";
-        displayedInputText = result;
+        settings.displayedInputText = result;
     }
 
     PageStack {
@@ -403,9 +406,9 @@
                             iconName: "edit"
                             text: i18n.tr("Edit")
                             onTriggered: {
-                                longFormula = model.formula;
-                                shortFormula =  model.result;
-                                displayedInputText = model.formula;
+                                settings.longFormula = model.formula;
+                                settings.shortFormula =  model.result;
+                                settings.displayedInputText = model.formula;
                                 isLastCalculate = false;
                                 previousVisual = "";
                                 scrollableView.scrollToBottom();
@@ -608,7 +611,7 @@
                             }
                         }
 
-                        text: Formula.returnFormulaToDisplay(displayedInputText)
+                        text: Formula.returnFormulaToDisplay(settings.displayedInputText)
                         font.pixelSize: height * 0.7
                         horizontalAlignment: TextInput.AlignRight
                         anchors {
@@ -693,10 +696,10 @@
                             if (cursorPosition !== length ) {
                                 // Count cursor position from the end of line
                                 var preservedCursorPosition = length - cursorPosition;
-                                displayedInputText = longFormula;
+                                settings.displayedInputText = settings.longFormula;
                                 cursorPosition = length - preservedCursorPosition;
                             } else {
-                                displayedInputText = shortFormula;
+                                settings.displayedInputText = settings.shortFormula;
                             }
 
                         SequentialAnimation {

=== modified file 'debian/changelog'
--- debian/changelog	2015-11-28 20:44:02 +0000
+++ debian/changelog	2015-11-28 23:18:30 +0000
@@ -5,6 +5,7 @@
   * Upgrade math.js to version 2.4.2 to fix complex numbers formatting 
   * Run Calculator in Landscape mode for Desktop (LP: #1468663)
   * Add instruction how to enable profiling (workaround for LP: #1520551) 
+  * Remember formula from InputField after application close (LP: #1520508)
 
  -- Bartosz Kosiorek <[e-mail]>  Thu, 12 Nov 2015 13:28:29 +0100