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
=== modified file 'Simple/CalcKeyboard.qml'
--- Simple/CalcKeyboard.qml 2014-01-23 18:34:48 +0000
+++ Simple/CalcKeyboard.qml 2014-03-10 18:41:03 +0000
@@ -278,10 +278,10 @@
278 id: pointButton278 id: pointButton
279 x: (calcGridUnit*11)*2279 x: (calcGridUnit*11)*2
280 y: (calcGridUnit*8)*4280 y: (calcGridUnit*8)*4
281 text: "."281 text: separator
282 onClicked: {282 onClicked: {
283 if (!hasToAddDot) { // To avoid multiple dots283 if (!hasToAddDot) { // To avoid multiple dots
284 hasToAddDot = formulaPush(text);284 hasToAddDot = formulaPush('.'); // Engine uses dot, but on screen there is local separator
285 }285 }
286 }286 }
287 }287 }
288288
=== modified file 'Simple/SimplePage.qml'
--- Simple/SimplePage.qml 2014-03-09 21:53:29 +0000
+++ Simple/SimplePage.qml 2014-03-10 18:41:03 +0000
@@ -26,6 +26,7 @@
26 property var screenFormula: [{_text:'', _operation: '', _number:''}]26 property var screenFormula: [{_text:'', _operation: '', _number:''}]
27 property bool labelVisible: false27 property bool labelVisible: false
28 property bool saveLabel: true28 property bool saveLabel: true
29 property var separator: Qt.locale().decimalPoint
29 /*30 /*
30 * hasToAddDot became true if dot is last pressed button.31 * hasToAddDot became true if dot is last pressed button.
31 * It is necessary only for +/-32 * It is necessary only for +/-
@@ -62,6 +63,9 @@
62 if (last !== undefined && last !== 'invalidOperation') { // Just to check it is not blank and if is a valid operation63 if (last !== undefined && last !== 'invalidOperation') { // Just to check it is not blank and if is a valid operation
63 // If the element is a number it has to be added to current line64 // If the element is a number it has to be added to current line
64 if (last.type === CALC.T_NUMBER || last.type === CALC.T_UNARY_MINUS) {65 if (last.type === CALC.T_NUMBER || last.type === CALC.T_UNARY_MINUS) {
66 if (separator !== '.' && last.value.toString() === '.') {
67 last.value = separator;
68 }
65 screenFormula[screenFormula.length - 1]._number += last.value.toString();69 screenFormula[screenFormula.length - 1]._number += last.value.toString();
66 added = true;70 added = true;
67 } else if (last.type === CALC.T_MINUS && screenFormula[screenFormula.length - 1]._number === ''){71 } else if (last.type === CALC.T_MINUS && screenFormula[screenFormula.length - 1]._number === ''){
@@ -101,6 +105,9 @@
101 // CALC.PRECISION is set in engine.js105 // CALC.PRECISION is set in engine.js
102 if (result) {106 if (result) {
103 result = result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"");107 result = result.toPrecision(CALC.PRECISION).replace(/\.0+$/,"");
108 if (separator !== '.' && result.indexOf('.') !== -1) {
109 result = result.replace(".", separator);
110 }
104 screenFormula.push({_text:'', _operation: '=', _number: result.toString()});111 screenFormula.push({_text:'', _operation: '=', _number: result.toString()});
105 }112 }
106 formulaView.currentOperatorsChanged();113 formulaView.currentOperatorsChanged();
@@ -232,10 +239,11 @@
232 event.text === "-" ||239 event.text === "-" ||
233 event.text === "*" ||240 event.text === "*" ||
234 event.text === "/" ||241 event.text === "/" ||
235 event.text === "." ||
236 event.text === "=") &&242 event.text === "=") &&
237 event.text !== "") {243 event.text !== "") {
238 buttonClicked(event.text)244 buttonClicked(event.text)
245 } else if (event.text === separator) {
246 buttonClicked('.');
239 }247 }
240 }248 }
241 else {249 else {
@@ -257,14 +265,15 @@
257 } else if (event.key === Qt.Key_Enter || event.key === Qt.Key_Return || event.key === Qt.Key_Equal) {265 } else if (event.key === Qt.Key_Enter || event.key === Qt.Key_Return || event.key === Qt.Key_Equal) {
258 buttonReleased('=');266 buttonReleased('=');
259 } else if ((!isNaN(event.text) ||267 } else if ((!isNaN(event.text) ||
260 event.text === "+" ||268 event.text === "+" ||
261 event.text === "-" ||269 event.text === "-" ||
262 event.text === "*" ||270 event.text === "*" ||
263 event.text === "/" ||271 event.text === "/" ||
264 event.text === "." ||272 event.text === "=") &&
265 event.text === "=") &&273 event.text !== "") {
266 event.text !== "") {
267 buttonReleased(event.text)274 buttonReleased(event.text)
275 } else if (event.text === separator) {
276 buttonReleased('.');
268 }277 }
269 }278 }
270 }279 }

Subscribers

People subscribed via source and target branches