Merge lp:~sinabakh/ubuntu-calculator-app/history-using-arrow-keys into lp:ubuntu-calculator-app

[Sina Bakhtiari]

Allow navigating the history using the arrow keys

[Bartosz Kosiorek (gang65)]

Thanks. Nice improvement.

Unfortunately it is causing lost of last typed formula.
Steps to reproduce:
1. Create several entries in calculation history
2. Type some formula: 25*25
2. Press up key (the previous calculation will appear)
3. Press down key

Currently:
- Empty formula is shown

Expected:
- 25*25 should be shown

review: Needs Fixing

[Bartosz Kosiorek (gang65)]

It will be also great to show some indicator what is currently selected formula.
It could be in form of background color.

Also I would expect that if any other than "up" and "down" key were pressed, then the formula selection were reset (it is starting from bottom).

review: Needs Fixing

[Sina Bakhtiari (sinabakh)]

> Thanks. Nice improvement.
>
> Unfortunately it is causing lost of last typed formula.
> Steps to reproduce:
> 1. Create several entries in calculation history
> 2. Type some formula: 25*25
> 2. Press up key (the previous calculation will appear)
> 3. Press down key
>
> Currently:
> - Empty formula is shown
>
> Expected:
> - 25*25 should be shown

Yes, currently it just uses the history of calculated formulas not the one which is being written. I will try to fix it.
Thanks for pointing it out.

[Sina Bakhtiari (sinabakh)]

The first problem that you mentioned is fixed.
Regarding the 2 next points you made:

> Also I would expect that if any other than "up" and "down" key were pressed,
> then the formula selection were reset (it is starting from bottom).

I think it might be better not to implement this, because there are cases where people want to access a formula in the history but edit parts of it. In that case they will use left/right keys to change the position of the cursor and other keys to edit the formula.

> It will be also great to show some indicator what is currently selected
> formula.
> It could be in form of background color.

Wouldn't it be better to implement this features in different branch ? I mean registering a different Bug and implement it independent of this history related branch. Because it have things more related to UI than the history logics.

[Bartosz Kosiorek (gang65)]

I have submitted ticket for left and right cursor:
https://bugs.launchpad.net/ubuntu-calculator-app/+bug/1610207

> The first problem that you mentioned is fixed.
> Regarding the 2 next points you made:
>
> > Also I would expect that if any other than "up" and "down" key were pressed,
> > then the formula selection were reset (it is starting from bottom).

Let's leave it as it is already implemented, and get UX feedback

> I think it might be better not to implement this, because there are cases
> where people want to access a formula in the history but edit parts of it. In
> that case they will use left/right keys to change the position of the cursor
> and other keys to edit the formula.
>
> > It will be also great to show some indicator what is currently selected
> > formula.
> > It could be in form of background color.
>
> Wouldn't it be better to implement this features in different branch ? I mean
> registering a different Bug and implement it independent of this history
> related branch. Because it have things more related to UI than the history
> logics.

I don't think this work could be separated, as it is stricly connected with "up and down" feature. Without indicator of currently selected calculation from history, it gives bad User Excperience.
For example user could press accidently "up" key, and the whole formula which was typed will gone. With indicator it will be visible that formula was changed with calculation from the history.

It will be also great to implement "Escape" press. If you press Escape key, then it exit from history preview and go back to the old formula.

What do you think about that?

[Sina Bakhtiari (sinabakh)]

> Let's leave it as it is already implemented, and get UX feedback
>
Sure, thanks!

>
> I don't think this work could be separated, as it is stricly connected with
> "up and down" feature. Without indicator of currently selected calculation
> from history, it gives bad User Excperience.
> For example user could press accidently "up" key, and the whole formula which
> was typed will gone. With indicator it will be visible that formula was
> changed with calculation from the history.

Yes, you are completely right.
Just a question, in you opinion, when user is browsing history and starts to edit one of past formulas, is it better to reset the history and take the edited formula as the current formula, or no, save the current state of history and do not overwrite the last formula (like what happens in terminal) ?

> It will be also great to implement "Escape" press. If you press Escape key,
> then it exit from history preview and go back to the old formula.
>
> What do you think about that?
I like the idea, it will be implemented.

P.S: Is there a place (IRC or mailing list) where I can reach you or other calculator developers in case that I have development questions?

Regards

[Bartosz Kosiorek (gang65)]

> Just a question, in you opinion, when user is browsing history and starts to
> edit one of past formulas, is it better to reset the history and take the
> edited formula as the current formula, or no, save the current state of
> history and do not overwrite the last formula (like what happens in terminal)
> ?

I would prefer to reset history and take the edited formula as the current formula. We cannot save every formula in history, as our assumption was that it should be mathematically correct (e.g. "100-" or "100*("

> > It will be also great to implement "Escape" press. If you press Escape key,
> > then it exit from history preview and go back to the old formula.
> >
> > What do you think about that?
> I like the idea, it will be implemented.

Great. We will be closer to use calculator only via keyboard.

> P.S: Is there a place (IRC or mailing list) where I can reach you or other
> calculator developers in case that I have development questions?

Some more information you could find on:
https://developer.ubuntu.com/en/community/core-apps/calculator/

[Bartosz Kosiorek (gang65)]

Let's merge it for now.

review: Approve

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

FAILED: Autolanding.
No commit message was specified in the merge proposal. Hit 'Add commit message' on the merge proposal web page or follow the link below. You can approve the merge proposal yourself to rerun.
https://code.launchpad.net/~sinabakh/ubuntu-calculator-app/history-using-arrow-keys/+merge/301567/+edit-commit-message

review: Needs Fixing (continuous-integration)

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

review: Approve (continuous-integration)

=== modified file 'app/ubuntu-calculator-app.qml'
--- app/ubuntu-calculator-app.qml	2016-01-18 23:40:25 +0000
+++ app/ubuntu-calculator-app.qml	2016-08-03 17:03:28 +0000
@@ -64,6 +64,15 @@
     // Var used to save favourite calcs
     property bool isFavourite: false
 
+    // Var used to store calculation history position
+    property var historyPosition: calculationHistory.getContents().count;
+
+    // Var used to save if user is using history.
+    property bool isUsingHistory: false;
+
+    // Var used to store the last formula which is being written.
+    property string lastWrittenFormula: "";
+
     // Var used to store currently edited calculation history item
     property int editedCalculationIndex: -1
 
@@ -312,7 +321,7 @@
 
             // Some special keys like backspace captured in TextField,
             // are for some reason not sent to the application but to the text input
-            Keys.onPressed: textInputField.keyPress(event)
+            Keys.onPressed: {event.accepted = true; textInputField.keyPress(event)}
             Keys.onReleased: textInputField.keyRelease(event)
 
             head.visible: false
@@ -618,6 +627,24 @@
 
                         function keyPress(event) {
                             if (!(event.modifiers & Qt.ControlModifier || event.modifiers & Qt.AltModifier)) { // Shift needs to be passed through as it may be required for some special keys
+                                if((event.key === Qt.Key_Up || event.key === Qt.Key_Down) && event.accepted) {
+                                    if(event.key === Qt.Key_Up && historyPosition > 1)
+                                        historyPosition--;
+                                    if(event.key === Qt.Key_Down && historyPosition < calculationHistory.getContents().count)
+                                        historyPosition++;
+                                    if(historyPosition !== calculationHistory.getContents().count) {
+                                        isUsingHistory = true;
+                                        clearFormula();
+                                        formulaPush(calculationHistory.getContents().get(historyPosition).formula);
+                                    }
+                                    else if(isUsingHistory)
+                                    {
+                                        clearFormula();
+                                        formulaPush(lastWrittenFormula);
+                                        isUsingHistory = false;
+                                    }
+                                }
+
                                 keyboardLoader.item.pressedKey = event.key;
                                 keyboardLoader.item.pressedKeyText = event.text;
                             } else if (event.modifiers & Qt.ControlModifier) {
@@ -697,6 +724,12 @@
                                 displayedInputText = shortFormula;
                             }
 
+                        onTextChanged: {
+                            if(! isUsingHistory) {
+                                lastWrittenFormula = textInputField.text;
+                            }
+                        }
+
                         SequentialAnimation {
                             id: errorAnimation
                             running: false

=== modified file 'app/ui/KeyboardPage.qml'
--- app/ui/KeyboardPage.qml	2015-11-09 14:18:18 +0000
+++ app/ui/KeyboardPage.qml	2016-08-03 17:03:28 +0000
@@ -154,6 +154,7 @@
                                 break;
                             case "calculate":
                                 calculate();
+                                historyPosition = calculationHistory.getContents().count;
                                 scrollableView.scrollToBottom();
                                 break;
                             }