Ubuntu Calculator App

Merge lp:~rpadovani/ubuntu-calculator-app/1287340 into lp:~ubuntu-calculator-dev/ubuntu-calculator-app/old_trunk

  1. 1287340
  2. Merge into old_trunk
Proposed by Riccardo Padovani
Status: Merged
Approved by: Riccardo Padovani
Approved revision: 227
Merged at revision: 232
Proposed branch: lp:~rpadovani/ubuntu-calculator-app/1287340
Merge into: lp:~ubuntu-calculator-dev/ubuntu-calculator-app/old_trunk
Diff against target: 84 lines (+19/-10)
2 files modified
Simple/CalcKeyboard.qml (+2/-2)
Simple/SimplePage.qml (+17/-8)
To merge this branch: bzr merge lp:~rpadovani/ubuntu-calculator-app/1287340
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Mihir Soni Approve
Bartosz Kosiorek Approve
Review via email: mp+209411@code.launchpad.net

Commit message

Fixed #1287340

Description of the change

Fixed #1287340

Now separator is localized, so keyboard shortcut

To post a comment you must log in.
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Thanks Riccardo for patch.

It works perfectly for me. Numpad works as it should.

I found one small visual issue: When you press "." on numpad the "dot button" on screen is always pressing down

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

Thanks Bartosz, nice catch.
I should have fixed it, could you confirm, please?

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

One more comment:

Instead of replacing:
  result = result.replace(".", separator);
you could use toLocaleString without checking
  result = result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"").toLocaleString(Qt.locale(), "f", 0);

Instead of replacing separators,
            function buttonClicked(buttonName) {
                if (buttonName === separator) {
                    buttonName = '.';
                }
                keyboardButtons[buttonName].clicked();
            }

            function buttonReleased(buttonName) {
                if (buttonName === separator) {
                    buttonName = '.';
                }
                keyboardButtons[buttonName].released();
            }

try to use contruction:
  Keys.onPressed: {
...
  Keys.onReleased: {
...

     else if (event.text === separator) {
        buttonClicked(".")

Instead of
                if (separator !== '.' && last.value.toString() === '.') {
                    last.value = separator;
                }
                screenFormula[screenFormula.length - 1]._number += last.value.toString();

use:
            screenFormula[screenFormula.length - 1]._number += last.value.toLocaleString(Qt.locale(), "f", 0);

Thanks !

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:225
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/160/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1576
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-raring-amd64-ci/159
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-saucy-amd64-ci/159
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-trusty-amd64-ci/31

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/160/rebuild

review: Approve (continuous-integration)
Revision history for this message
Riccardo Padovani (rpadovani) wrote :

> One more comment:
>
> Instead of replacing:
> result = result.replace(".", separator);
> you could use toLocaleString without checking
> result = result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"").toLocaleStri
> ng(Qt.locale(), "f", 0);

Doesn't work (here in Italy we use , as separator):
console.log('result: ' + result);
console.log('result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"").toLocaleString(Qt.locale(), "f", 0): ' + result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"").toLocaleString(Qt.locale(), "f", 0));
console.log("Qt.locale().decimalPoint " + Qt.locale().decimalPoint);

result: 12.5
result.toPrecision(CALC.PRECISION).replace(/.0+$/,"").toLocaleString(Qt.locale(), "f", 0): 12.500000
Qt.locale().decimalPoint ,

> Instead of replacing separators,
> function buttonClicked(buttonName) {
> if (buttonName === separator) {
> buttonName = '.';
> }
> keyboardButtons[buttonName].clicked();
> }
>
> function buttonReleased(buttonName) {
> if (buttonName === separator) {
> buttonName = '.';
> }
> keyboardButtons[buttonName].released();
> }
>
> try to use contruction:
> Keys.onPressed: {
> ...
> Keys.onReleased: {
> ...
>
> else if (event.text === separator) {
> buttonClicked(".")

Good idea, done!

> Instead of
> if (separator !== '.' && last.value.toString() === '.') {
> last.value = separator;
> }
> screenFormula[screenFormula.length - 1]._number +=
> last.value.toString();
>
> use:
> screenFormula[screenFormula.length - 1]._number +=
> last.value.toLocaleString(Qt.locale(), "f", 0);

Doesn't work:

console.log("last.value: " + last.value);
console.log("last.value.toLocaleString(Qt.locale(), 'f', 0): " + last.value.toLocaleString(Qt.locale(), "f", 0));
console.log("Qt.locale().decimalPoint: " + Qt.locale().decimalPoint);

last.value: .
last.value.toLocaleString(Qt.locale(), 'f', 0): .
Qt.locale().decimalPoint: ,

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:226
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/161/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1580
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-raring-amd64-ci/160
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-saucy-amd64-ci/160
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-trusty-amd64-ci/32

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/161/rebuild

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

PASSED: Continuous integration, rev:227
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/162/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1583
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-raring-amd64-ci/161
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-saucy-amd64-ci/161
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-trusty-amd64-ci/33

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calculator-app-ci/162/rebuild

review: Approve (continuous-integration)
Revision history for this message
Bartosz Kosiorek (gang65) :
review: Approve
Revision history for this message
Mihir Soni (mihirsoni) wrote :

Looks good.
Thank you Riccardo :)

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

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/ubuntu-calculator-app-autolanding/71/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1653/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calculator-app-raring-amd64-autolanding/71/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calculator-app-saucy-amd64-autolanding/71/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calculator-app-trusty-amd64-autolanding/27/console

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

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/ubuntu-calculator-app-autolanding/74/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1669
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-raring-amd64-autolanding/74
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-saucy-amd64-autolanding/74
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-trusty-amd64-autolanding/30

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

FAILED: Autolanding.
Unapproved changes made after approval.
http://91.189.93.70:8080/job/ubuntu-calculator-app-autolanding/75/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/1671
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-raring-amd64-autolanding/75
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-saucy-amd64-autolanding/75
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calculator-app-trusty-amd64-autolanding/31

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Simple/CalcKeyboard.qml'
2--- Simple/CalcKeyboard.qml 2014-01-23 18:34:48 +0000
3+++ Simple/CalcKeyboard.qml 2014-03-10 18:41:03 +0000
4@@ -278,10 +278,10 @@
5 id: pointButton
6 x: (calcGridUnit*11)*2
7 y: (calcGridUnit*8)*4
8- text: "."
9+ text: separator
10 onClicked: {
11 if (!hasToAddDot) { // To avoid multiple dots
12- hasToAddDot = formulaPush(text);
13+ hasToAddDot = formulaPush('.'); // Engine uses dot, but on screen there is local separator
14 }
15 }
16 }
17
18=== modified file 'Simple/SimplePage.qml'
19--- Simple/SimplePage.qml 2014-03-09 21:53:29 +0000
20+++ Simple/SimplePage.qml 2014-03-10 18:41:03 +0000
21@@ -26,6 +26,7 @@
22 property var screenFormula: [{_text:'', _operation: '', _number:''}]
23 property bool labelVisible: false
24 property bool saveLabel: true
25+ property var separator: Qt.locale().decimalPoint
26 /*
27 * hasToAddDot became true if dot is last pressed button.
28 * It is necessary only for +/-
29@@ -62,6 +63,9 @@
30 if (last !== undefined && last !== 'invalidOperation') { // Just to check it is not blank and if is a valid operation
31 // If the element is a number it has to be added to current line
32 if (last.type === CALC.T_NUMBER || last.type === CALC.T_UNARY_MINUS) {
33+ if (separator !== '.' && last.value.toString() === '.') {
34+ last.value = separator;
35+ }
36 screenFormula[screenFormula.length - 1]._number += last.value.toString();
37 added = true;
38 } else if (last.type === CALC.T_MINUS && screenFormula[screenFormula.length - 1]._number === ''){
39@@ -101,6 +105,9 @@
40 // CALC.PRECISION is set in engine.js
41 if (result) {
42 result = result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"");
43+ if (separator !== '.' && result.indexOf('.') !== -1) {
44+ result = result.replace(".", separator);
45+ }
46 screenFormula.push({_text:'', _operation: '=', _number: result.toString()});
47 }
48 formulaView.currentOperatorsChanged();
49@@ -232,10 +239,11 @@
50 event.text === "-" ||
51 event.text === "*" ||
52 event.text === "/" ||
53- event.text === "." ||
54 event.text === "=") &&
55 event.text !== "") {
56 buttonClicked(event.text)
57+ } else if (event.text === separator) {
58+ buttonClicked('.');
59 }
60 }
61 else {
62@@ -257,14 +265,15 @@
63 } else if (event.key === Qt.Key_Enter || event.key === Qt.Key_Return || event.key === Qt.Key_Equal) {
64 buttonReleased('=');
65 } else if ((!isNaN(event.text) ||
66- event.text === "+" ||
67- event.text === "-" ||
68- event.text === "*" ||
69- event.text === "/" ||
70- event.text === "." ||
71- event.text === "=") &&
72- event.text !== "") {
73+ event.text === "+" ||
74+ event.text === "-" ||
75+ event.text === "*" ||
76+ event.text === "/" ||
77+ event.text === "=") &&
78+ event.text !== "") {
79 buttonReleased(event.text)
80+ } else if (event.text === separator) {
81+ buttonReleased('.');
82 }
83 }
84 }

Subscribers

People subscribed via source and target branches