Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Conversation

@cafca
Copy link
Member

@cafca cafca commented Apr 3, 2022

  • adds conversion from json to cbor
  • conversion/validation is called whenever textbox contents are changed
    • removes conversion/validation buttons
    • removes window alert and displays error messages in the text boxes

@adzialocha
Copy link
Member

adzialocha commented Apr 4, 2022

adds conversion from json to cbor

The conversion from JSON to CBOR is super cool!

I've changed the debounce code as one needs define the to-be-called method only once to assure it is working (you created a new definition as soon as there was a keyup event taking place). Here is where I took the code from btw: https://www.freecodecamp.org/news/javascript-debounce-example/

conversion/validation is called whenever textbox contents are changed

Regarding the automatic changes of data as soon as I type something: First I thought "woah, cool", then I played with it a little and stumbled upon a few things I'm not sure I like them: a) If I manually write JSON it makes a dozen of invalid requests to the server before I finished writing valid input b) I have to change something in the already complete CDDL to re-trigger the validation after changing the JSON or CBOR c) the error messages overwrite my CBOR / JSON input now while I was writing it.

I think its totally fine to trigger requests pressing the buttons, or if we're keen on using key events, then I'd come up with better places where to display error messages, also trigger CDDL validation and maybe validate the JSON before we send it to the server? Since this is just a simple tool I'd be happy to not over-engineer this, take this nice new feature and get back to the buttons as they just work without surprises 😄

@cafca
Copy link
Member Author

cafca commented Apr 5, 2022

Super, thanks. I was stuck with the debounce. The logic around the live conversion can be improved, yes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants