Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic UI #14

Merged
merged 18 commits into from
Jul 17, 2023
Merged

Dynamic UI #14

merged 18 commits into from
Jul 17, 2023

Conversation

mariopil
Copy link
Member

No description provided.

@mariopil mariopil self-assigned this Jul 12, 2023
@mariopil mariopil marked this pull request as draft July 12, 2023 09:38
@mariopil mariopil requested review from geofmureithi and eloylp July 12, 2023 09:38
@mariopil mariopil marked this pull request as ready for review July 13, 2023 09:58
Copy link
Member

@eloylp eloylp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job ! Things that i checked:

  • Tried the UI locally. UI works with both contracts Lottery and Voting, correctly replacing all values in the CLI command template when pressing the invoke button.
  • Code in general, i think is good for the demo.
  • That the targets for the first demo are met (10% of the first milestone)
  • Just missing the information for the readme but maybe this is for later.

</Modal.Header>
<Modal.Body>
<p>
<pre>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a nitpick, would it be possible to add a more "terminal-like" style to the invoke modal ? Currently is in this way:

image

By adding this style property (playing around):

<pre style={{ backgroundColor: 'black' }}>

I was able to get this :) :

image

I like it more let me know what do you think. Also, i am wondering if the typical button of "copy to clipboard" would be easy to add.

Thanks for the great work 😍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it. As for the button - it is there and it works :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, you mean this one right ? for copying the contract code:

image

I am suggesting to add the same, but in the invoke modal. So the user can copy the CLI invokation also :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, maybe needs a bit more of elaboration as, with the code i shared before, affects every modal ofc .....

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both the background and button.

Copy link
Member

@eloylp eloylp Jul 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you ! just a last thing i realised when copying the command into the terminal. We need the \ for the new lines 😝 . I took the liberty of pushing this commit df7125b , as i cannot send to you a patch over here (github does not allow patch file extension). Now looks fixed:

image

But let me know if you agree with it :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! :)

Copy link
Contributor

@geofmureithi geofmureithi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mariopil mariopil merged commit d1d4570 into main Jul 17, 2023
@mariopil mariopil deleted the dynamic-ui branch July 17, 2023 06:44
eloylp added a commit that referenced this pull request Jul 21, 2023
* Created initial UI with boostrap.

* Reorganized UI, added settings and generated code for Lottery contract.

* Clean up.

* Code reformat.

* Replaced the static UI with the dynamic one made in React.

* Small layout change of buttons.

* Code reformat

* Revert "Auxiliary commit to revert individual files from 0645906"

This reverts commit 38f538517f766606cb3858317f125c7281324329.

* Code reformat with linter.

* More reformat

* Added navbar and readme page.

* Code cleanup

* Fixed prettier warnings, removed eslint flag to warn of space missing before parenthesis

* Removed from eslint checking for trailing comma

* Added black background 'copy to clipboard' button to invoke code dialog

* Fix line breaks in invoke CLI commands

---------

Co-authored-by: eloylp <eloylp@eloylp.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants