Add Calculator demo #174

OhadRau · 2019-01-02T00:07:49Z

Still a few things left to do, but this is a mostly working calculator.

Stuff (maybe) left to go:

Make operators show up in the display window before evaluation
Change font so we can have a backspace symbol
Figure out how to get relative sizing to work (is this possible with Revery yet?)
Scale font size along with buttons so that it doesn't look tiny (maybe?)

I'm also not sure how great my use of state was here, but I guess since we don't do state the same way as ReasonReact (i.e. state+action) this would be pretty typical of a Revery app. Regardless, would like any suggestions for how to make this easier to understand or improve the code in general.

OhadRau · 2019-01-03T06:39:36Z

Relative spacing is currently really weird to do with the <Clickable> tag, since we can't override it's style. We might want to block this PR on a change that'll allow us to override the Clickable's flex-grow and such. With flex-grow on the <view> inside the button we grow vertically. flex-grow on a view outside almost works, but the clickable area stays within the <Clickable> tag which isn't growing horizontally.

bryphe · 2019-01-03T18:09:15Z

@OhadRau - this is awesome work! It's great to finally see an interactive demo come together.

Testing on Windows:

Like, it feels like you could build a usable app with this now! So cool!

I'm also not sure how great my use of state was here, but I guess since we don't do state the same way as ReasonReact (i.e. state+action) this would be pretty typical of a Revery app. Regardless, would like any suggestions for how to make this easier to understand or improve the code in general.

I think the way you used state here is perfectly appropriate (and highlights the usage of hooks within revery). We do have a concept of a redux-like state store, but I don't think it's necessary to use that in here. The nice thing about encapsulating the state the way that you did is the <Calculator /> could be easily extracted as a component and embedded into other apps. It might be the case that we don't need the concept of an app-wide state store at all, and can just delegate to useReducer/useState. (One thing I'm interested in is introducing time-travel debugging like the redux dev tools have - but I'm thinking that, with some of the thinking @cristianoc and @jchavarri have done on revery-ui/reason-reactify#47, we could get that for component state too. It's totally a tangent so I'll log a separate issue to track this).

Relative spacing is currently really weird to do with the tag, since we can't override it's style. We might want to block this PR on a change that'll allow us to override the Clickable's flex-grow and such.

Good point. This was an oversight by me, we should have a style property exposed on <Clickable />. I'm happy to either bring this PR in as-is (and then we can fix <Clickable /> but update this example), or fix <Clickable /> prior to merging this - whatever you'd prefer @OhadRau .

Sorry about the CI build issues - fixing this in #178

Awesome example and thanks again! 👍

bryphe · 2019-01-03T18:16:35Z

Change font so we can have a backspace symbol

For this, we could bring in font-awesome, and render the backspace icon: https://fontawesome.com/icons/backspace?style=solid

I had some prototype code for this that I just ported over to the Oni2 repo: onivim/oni2@2260c99

bryphe

Code looks good to me! I'm happy to tackle the remaining bullet points in separate PRs - whatever you'd prefer, @OhadRau .

bryphe · 2019-01-03T18:43:15Z

(Also, if you bring in latest master, the build should succeed)

OhadRau · 2019-01-03T19:09:24Z

Awesome, I believe that @bmathews was working on exactly that in his clickable-styles branch, so I guess I'll just wait until we have that merged in (unless you'd prefer to have this in master now so we have an interactive demo).

bmathews · 2019-01-03T19:34:55Z

@OhadRau @bryphe

That is what I was going for, yep, but I'm not a reason expert so there might be a better way to do it:

https://github.com/revery-ui/revery/compare/master...bmathews:clickable-styles?expand=1

jchavarri · 2019-01-03T19:47:24Z

Hey @OhadRau @bryphe I was looking at this calculator example (which is awesome!! 🚀 ) and wanted to explore how useReducer would look in this case, on one side because it's such a exemplary "state machine" case, plus it was a good chance to test that API that was added recently.

I created a PR from this branch in https://github.com/OhadRau/revery/pull/1/files but this is the gist of the different types:

type state = {
  operator, /* Current operator being applied */
  result: float, /* The actual numerical result */
  display: string, /* The equation displayed */
  number: string /* Current number being typed */
};

type action =
  | BackspaceKeyPressed
  | ClearKeyPressed(bool) /*AC pressed */
  | DotKeyPressed
  | NumberKeyPressed(string)
  | OperationKeyPressed(operator)
  | PlusMinusKeyPressed
  | ResultKeyPressed;

then the reducer (i'm skipping the eval function which would be almost 1:1 what exists already):

let reducer = (state, action) =>
  switch (action) {
  | BackspaceKeyPressed =>
    state.number == "" ?
      state :
      {
        ...state,
        number: String.sub(state.number, 0, String.length(state.number) - 1),
      }
  | ClearKeyPressed(ac) =>
    ac ?
      {...state, operator: `Nop, result: 0., display: ""} :
      {...state, number: ""}
  | DotKeyPressed =>
    String.contains(state.number, '.') ?
      state : {...state, number: state.number ++ "."}
  | NumberKeyPressed(n) => {...state, number: state.number ++ n}
  | OperationKeyPressed(o) =>
    let (result, display) = eval(state, o);
    {operator: o, result, display, number: ""};
  | PlusMinusKeyPressed =>
    if (state.number != "" && state.number.[0] == '-') {
      {
        ...state,
        number: String.sub(state.number, 1, String.length(state.number) - 1),
      };
    } else {
      {...state, number: "-" ++ state.number};
    }
  | ResultKeyPressed =>
    let (result, display) = eval(state, `Nop);
    {operator: `Nop, result, display, number: showFloat(result)};
  };

and finally the consumption in the component:

let ({display, number, _}, dispatch) =
  useReducer(
    reducer,
    {operator: `Nop, result: 0., display: "", number: ""},
  );

I think either useState or useReducer are fine, we can add useReducer to another example in the future 😄 Let me know what you think!

OhadRau · 2019-01-03T19:49:39Z

Yeah that actually looks good to me @bmathews. In terms of functions to extend style that's probably the cleanest we can get it. The only alternative I can really think of would be to define styles without the Style.make function and to use the {...spread} syntax for records, but that's up to the user to do on their own.

OhadRau · 2019-01-03T19:52:43Z

@jchavarri Awesome! Didn't realize we have a useReducer function but I think that'll actually help me clean up the current code a lot. Since that's not in any examples already this could be a good place to add it.

useReducer in calculator

bryphe · 2019-01-04T00:41:28Z

examples/Calculator.re

+
+              | _ => ()
+            };
+            /* TODO: Pretty sure this isn't supposed to go in the render() function.


Good intuition, I think it is subscribing to the event on each re-render!

You could try this:

useEffect(() => { let unsubscribe = Event.subscribe(window.onKeyDown, respondToKeys); unsubscribe });

On each re-render, it will unsubscribe from the previous event subscription.

@bryphe Would this be a good opportunity to use the MountUnmount condition? Like in this test?

@jchavarri - I think it'd be reasonable (and better for perf)! Not sure if we'd hit that long-lived setState issue though in revery-ui/reason-reactify#44 ?

@bryphe Is useEffect exposed right now? I don't see it in Hooks and doesn't look like there's access to UiReact anywhere. Want me to add this to the PR?

Ah sorry - it wasn't exposed! Thanks for adding it @OhadRau 👍

bryphe · 2019-01-04T00:58:38Z

One other cool thing - with revery-ui/reason-fontkit#13, and aside from a minor glyph bug (revery-ui/reason-fontkit#14 - not related to this PR)... this calculator actually works and is usable in the browser too as a WebGL app! 😍

…culator

OhadRau · 2019-01-04T04:29:26Z

@bmathews Actually, one thing I just thought of for clickable-styles is that it probably makes the most sense to extend a transform by concatenating two lists. For example, if <Clickable> were to have a rotation applied by default, extending it with a scale transform would cancel that. Instead, we'd probably want to take the old style and add to that since that seems more akin to what CSS styles would do. Regardless, feel free to open it as a PR or issue here (even if it's not done yet) so we can centralize discussion on it and track progress.

OhadRau · 2019-01-07T03:00:24Z

Merged in @bmathews' clickable-styles branch & then used that to make the buttons scale. Might still investigate making the font size scale along with the buttons since it looks a little funny, but in the meantime would you mind reviewing this again since it's some new changes @bryphe?

bryphe · 2019-01-07T22:39:15Z

Just grabbed the latest! Code looks great, and I love the way calculator works now (handling key input is a big plus!). Thanks for the clickable-styles work too @bmathews !

I really like the first-class icon we have for backspace too. Consider me signed-off @OhadRau 👍

bryphe · 2019-01-08T00:14:10Z

Just checked the build - looks like it hit this error:

2019-01-08T00:05:40.8605848Z File "examples/dune", line 2, characters 4-227:
2019-01-08T00:05:40.8606706Z 2 |     (names Hello Autocomplete Flexbox Border Boxshadow DefaultButton Overflow)
2019-01-08T00:05:40.8607220Z 3 |     (preprocess (pps lwt_ppx))
2019-01-08T00:05:40.8607358Z 4 |     (package Revery)
2019-01-08T00:05:40.8607469Z 5 |     (public_names Hello Autocomplete Flexbox Border Boxshadow DefaultButton Calculator Overflow)

I fixed it inline - 🤞 it works!

OhadRau · 2019-01-08T00:26:56Z

Thanks for the fix! Looks like the CI isn't working again, any chance you can restart it or merge this in without? @bryphe

bryphe · 2019-01-08T01:20:39Z

Ah looks like there is still one more build failure:

File "src/UI/Style.re", line 122, characters 15-556:
Error: Some record fields are undefined: overflow

I pushed up a commit with the fix, you might just need to cherry-pick it (git cherry-pick 27970c37). Hopefully CI is green after that!

EDIT: Realized I could just update this line too - done - 🤞 the build is really green this time.

OhadRau · 2019-01-08T04:56:25Z

Merging now! Thanks for all the help

OhadRau and others added 6 commits January 1, 2019 14:34

Add proof of concept calculator demo

7ae1ee2

Fix some minor bugs

47927f0

Add style.extend

7c2b2dc

Use style.extend in Clickable

3d768dd

Use Clickable styles in Hello button example

68b46b3

Improve calculator display

0e21168

bryphe approved these changes Jan 3, 2019

View reviewed changes

Merge in upstream master

f970348

bmathews and others added 2 commits January 3, 2019 11:37

Update Hello.re

66829b2

useReducer in calculator

5c57b23

jchavarri mentioned this pull request Jan 3, 2019

useReducer in calculator OhadRau/revery#1

Merged

bmathews and others added 4 commits January 3, 2019 12:31

Update Hello.re

d5ea4a7

Reset number when clear key is pressed

240b993

Merge pull request #1 from jchavarri/calculator-reducer

acb5653

useReducer in calculator

Add (laggy) keyboard input

aa934b6

bryphe mentioned this pull request Jan 4, 2019

Bugfix: Glyph positioning in JS render strategy revery-ui/reason-fontkit#13

Merged

bryphe reviewed Jan 4, 2019

View reviewed changes

OhadRau added 3 commits January 3, 2019 20:33

Merge branch 'master' of https://github.com/revery-ui/revery into cal…

d65508c

…culator

Unsubscribe from keyboard

b0782b0

Add FontAwesome backspace

886d913

OhadRau force-pushed the calculator branch from 4771411 to 886d913 Compare January 4, 2019 03:57

OhadRau added 2 commits January 6, 2019 21:43

Merge bmathews/clickable-styles

e122d15

Make calculator buttons scale automatically

66cb1a0

Merge branch 'master' into calculator

6efbfb4

Update dune with Calculator executable in both places

3539bc3

Update Style.re - add overflow to Style.extend

c4f1b98

OhadRau merged commit d969ac3 into revery-ui:master Jan 8, 2019

OhadRau mentioned this pull request Jan 8, 2019

Example: Calculator #39

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Calculator demo #174

Add Calculator demo #174

OhadRau commented Jan 2, 2019 •

edited

Loading

OhadRau commented Jan 3, 2019

bryphe commented Jan 3, 2019

bryphe commented Jan 3, 2019

bryphe left a comment

bryphe commented Jan 3, 2019

OhadRau commented Jan 3, 2019 •

edited

Loading

bmathews commented Jan 3, 2019

jchavarri commented Jan 3, 2019

OhadRau commented Jan 3, 2019

OhadRau commented Jan 3, 2019

bryphe Jan 4, 2019

jchavarri Jan 4, 2019

bryphe Jan 4, 2019

OhadRau Jan 4, 2019

bryphe Jan 4, 2019

bryphe commented Jan 4, 2019

OhadRau commented Jan 4, 2019

OhadRau commented Jan 7, 2019

bryphe commented Jan 7, 2019

bryphe commented Jan 8, 2019 •

edited

Loading

OhadRau commented Jan 8, 2019

bryphe commented Jan 8, 2019 •

edited

Loading

OhadRau commented Jan 8, 2019

Add Calculator demo #174

Add Calculator demo #174

Conversation

OhadRau commented Jan 2, 2019 • edited Loading

OhadRau commented Jan 3, 2019

bryphe commented Jan 3, 2019

bryphe commented Jan 3, 2019

bryphe left a comment

Choose a reason for hiding this comment

bryphe commented Jan 3, 2019

OhadRau commented Jan 3, 2019 • edited Loading

bmathews commented Jan 3, 2019

jchavarri commented Jan 3, 2019

OhadRau commented Jan 3, 2019

OhadRau commented Jan 3, 2019

bryphe Jan 4, 2019

Choose a reason for hiding this comment

jchavarri Jan 4, 2019

Choose a reason for hiding this comment

bryphe Jan 4, 2019

Choose a reason for hiding this comment

OhadRau Jan 4, 2019

Choose a reason for hiding this comment

bryphe Jan 4, 2019

Choose a reason for hiding this comment

bryphe commented Jan 4, 2019

OhadRau commented Jan 4, 2019

OhadRau commented Jan 7, 2019

bryphe commented Jan 7, 2019

bryphe commented Jan 8, 2019 • edited Loading

OhadRau commented Jan 8, 2019

bryphe commented Jan 8, 2019 • edited Loading

OhadRau commented Jan 8, 2019

OhadRau commented Jan 2, 2019 •

edited

Loading

OhadRau commented Jan 3, 2019 •

edited

Loading

bryphe commented Jan 8, 2019 •

edited

Loading

bryphe commented Jan 8, 2019 •

edited

Loading