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

Hide UI inputs until contract is deployed #233

Conversation

nathan-lapinski
Copy link
Contributor

Description of the Change

This change updates some of the template app.js files to check if Contract.address is set before displaying any UI input elements. It also includes some html/css changes to hide input elements by default, and display a welcome message instead.

So far, I've only added updates to Hello World and Crypto Pizza. If the community is ok with the approach of this PR, I'd be happy to update the Coin template as well.

Alternate Designs

I'm not sure if checking Contract.address is the best way to determine whether or not the contract has been deployed. I was (and still am) looking to see if there's any kind of event or Promise I can listen to and be notified when the contract is deployed. If anyone knows of such a way, please let me know.

However, checking Contract.address appears to work, so I thought I'd submit here as a first attempt.

Benefits

Users will no longer see input fields that they can't yet interact with before deploying a contract. For example, in Hello World, entering a message and submitting would cause the app to crash (since this.instance was not yet fully defined)

Possible Drawbacks

It assumes that checking Contract.address is a reliable way of determining if the contract has been deployed. That could potentially make things brittle.

Verification Process

  • How did you verify that all new functionality works as expected?
    For both Hello World and Crypto Pizzas, I manually built the app locally and dev tested it.
    Hello World
  1. node scripts/generate-templates.js
  2. npm start
  3. Selected Hello World template, clicked Create Project
  4. UI inputs were hidden initially, and a welcome message was displayed.

Screen Shot 2020-05-14 at 21 23 36

  1. I clicked the deploy button
  2. The UI inputs were rendered, and the welcome message was hidden

7 . I entered "test message" in the "Update message" input, and clicked send
8. The update worked

Screen Shot 2020-05-14 at 22 00 06

Crypto Pizzas

  1. node scripts/generate-templates.js
  2. npm start
  3. Selected Crypto Pizzas NFT template, clicked Create Project
  4. UI inputs were hidden initially, and a welcome message was displayed.

Screen Shot 2020-05-14 at 22 01 45

  1. I clicked the deploy button

  2. The welcome message went away, and the UI inputs were rendered

  3. I entered "olives" into the "enter name..." input, and clicked "create". It was successful

Screen Shot 2020-05-14 at 22 03 04

  • How did you verify that all changed functionality works as expected?
    Manually built the app locally and dev tested it. Same steps as above

  • How did you verify that the change has not introduced any regressions?
    Other than manual testing of the app, I haven't really. Are the template files currently covered by tests? I ran into some issues trying to run tests from the project root, as well as the /editor root.
    Error: Cannot find module '../src/components/solc/dist/soljson-v0.5.10.js'

Github Issues

#228 #207

UI input elements.

Update some html/css to hide input elements by default,
and display a welcome message instead.
@nathan-lapinski nathan-lapinski changed the title [WIP] Hide UI inputs until contract is deployed Hide UI inputs until contract is deployed May 14, 2020
@samajammin
Copy link
Collaborator

Amazing! Thanks for this contribution @nathan-lapinski!

I'm having some issues setting up my local env (I believe the staging server is down ATM - please confirm @javier-tarazaga) but once that's sorted out I will definitely take a look at this!

@nathan-lapinski
Copy link
Contributor Author

@samajammin Thanks!

@samajammin
Copy link
Collaborator

This looks great @nathan-lapinski! Thanks again for this.

A couple small thoughts. I think it'd be even better to hide some of the text elements if the contract is not deployed. e.g. for the Hello World template:
Image 2020-05-28 at 1 32 33 PM
I think it makes sense to hide the "Message:" & "Block number:" texts until the contract is deployed. Perhaps we could add a "Welcome!" message in their place until the contract is deployed.

Same with CryptoPizza:
Image 2020-05-28 at 1 36 30 PM

I think it'd be best to also hide the navigation tab & "Create new CryptoPizza" until the text is deployed.

samajammin
samajammin previously approved these changes May 28, 2020
Copy link
Collaborator

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

This looks amazing! 😍

@nathan-lapinski would you mind making this same change to the Coin template as well?

@nathan-lapinski
Copy link
Contributor Author

@samajammin Thanks for the reviews and feedback!!
I will implement the UI changes that you have suggested, and will make the same changes to the Coin template as well, and add them to this PR. I should be able to complete it this weekend 👍

Ronhub88
Ronhub88 previously approved these changes May 31, 2020
@nathan-lapinski
Copy link
Contributor Author

Sorry, ended up being a little swamped this weekend. I'll try to have these changes up in the next couple of days 👍

@nathan-lapinski nathan-lapinski dismissed stale reviews from Ronhub88 and samajammin via 5c3d16e June 6, 2020 08:10
@nathan-lapinski nathan-lapinski changed the title Hide UI inputs until contract is deployed [WIP]Hide UI inputs until contract is deployed Jun 6, 2020
to follow a welcome content / main content structure.
@nathan-lapinski nathan-lapinski changed the title [WIP]Hide UI inputs until contract is deployed Hide UI inputs until contract is deployed Jun 7, 2020
@nathan-lapinski
Copy link
Contributor Author

@samajammin @RONIZAM80 I think this one is ready for another round of review 👍 Sorry for the delay.

I've updated the Coin template to display a welcome message until a contract is deployed.
I've also refactored all three template UI's, per @samajammin 's comments above. Now, only a welcome message will be displayed until a contract is deployed:

Screen Shot 2020-06-06 at 16 58 36

I've also split the app.html files into two sections:
<div id="welcome-container"> which displays until a contract is deployed, and
<div id="main-container" class="hidden">, which is initially hidden, and displays once a contract has been deployed.

Please let me know if there are any additional changes or updates that you'd like to see in this PR. Thanks in advance for the reviews!

Copy link
Collaborator

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Looks great to me! Nice work!

@samajammin
Copy link
Collaborator

@javier-tarazaga please give this a final review. I believe it's ready to ship 🚀

@javier-tarazaga javier-tarazaga merged commit 1647071 into SuperblocksHQ:master Jun 9, 2020
@javier-tarazaga
Copy link
Contributor

This looks awesome @nathan-lapinski !! Thank you so much for this :)

Let me give it a quick test in staging before promoting to prod!

Thanks once again!

@nathan-lapinski nathan-lapinski deleted the hide-ui-inputs-until-contract-deploys branch June 9, 2020 22:10
@nathan-lapinski
Copy link
Contributor Author

Thanks @javier-tarazaga and @samajammin !!

@samajammin
Copy link
Collaborator

@javier-tarazaga the changes in staging LGTM - can we push to prod?

@javier-tarazaga
Copy link
Contributor

@nathan-lapinski so finally the code is in prod! :)

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.

None yet

4 participants