ZeroPhone / ZPUI

Official ZeroPhone UI framework, based on pyLCI
http://zpui.rtfd.org/
Apache License 2.0
79 stars 19 forks source link

Basic Calculator App #112

Closed quinm0 closed 5 years ago

quinm0 commented 5 years ago

This is my first PR to this repo so please forgive me for everything you're about to witness. What I've made is a function baised calculator with 6 functions ( 7 if you count the init_app function ). There is one function do_operation that does all of the heavy lifting and another function get_numbers that obtains the two Decimals to operate on. The other 4 functions solely exist to call do_operation.

Note: Now that I'm making this PR it's hit me that I could probably add some arguments to the main_menu_contents array for the callback functions but alas I haven't read all of the documentation for ZPUI yet. So if this is the case, let me know and I'll fix the app.

Note Note: Also there's a few commits added to this branch that I don't know the origin of ( All of the commits that aren't mine ) , please let me know if this is an issue.

codecov[bot] commented 5 years ago

Codecov Report

Merging #112 into devel will increase coverage by 0.02%. The diff coverage is 34.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #112      +/-   ##
==========================================
+ Coverage   28.17%   28.19%   +0.02%     
==========================================
  Files         209      210       +1     
  Lines       16371    16423      +52     
==========================================
+ Hits         4613     4631      +18     
- Misses      11758    11792      +34
Impacted Files Coverage Δ
apps/calculator/main.py 34.61% <34.61%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update af8ce6d...55d5826. Read the comment docs.

CRImier commented 5 years ago

Now that I'm making this PR it's hit me that I could probably add some arguments to the main_menu_contents array for the callback functions but alas I haven't read all of the documentation for ZPUI yet. So if this is the case, let me know and I'll fix the app.

Yes, that's possible - you can get rid of 4 do_* functions that way. Do as follows:

["Add", lambda: do_operation('+')],

in the main_menu_contents.

quinm0 commented 5 years ago

That's awesome. I'll look into doing this soon ( hopefully ), but per your recommendation of making the app

more alike a typical calculator app which people are used to

I'm going to re-evaluate how I've built the app. I was looking into using eval() to do math with my original code but opted not to for the sake of simplicity. Now I just need to logistically figure out sanitization and other UX design and then I'll do my best to re-create the app with a more typicall calculator design.

CRImier commented 5 years ago

I think you using decimals library (as opposed to eval) is the best solution - as suggested in the original issue. As for sanitization - as I imagine it, the app will likely change quite a bit, so I'm not sure the code you might write now will still apply.

To be exact, I suggest we move away from using Menus&Printers and use Canvas to draw the main (and only) calculator screen, then process keypresses (arrow keys, F1/F2 and number keys) and redraw the canvas accordingly.

CRImier commented 5 years ago

Hi! Made the changes and merged the branch =) Will write a TODO for the calculator app today/tomorrow

CRImier commented 5 years ago

Done