CosmWasm/dApps

Fix up name service app

Closed this issue · 9 comments

ethanfrey commented

Here are a number of TODOs left after merging #13 You can tackle them as separate PRs or all at once:

  • Name validation doesn't check for lower case (try searching for SHELL and registering it, you get an error) - maybe better to fix after fixing the error display
  • There are no details on the error page. It looks nice, but can you add another section below with at least some dev-friendly message? (test this first, then fix validation)
    • I did see them full stack trace on the console, including "execute wasm contract failed: generic: Invalid character: 'S': failed to execute message; message index: 0"
    • You could search for "execute wasm contract failed: (.+); message index: (\d+)" and then display the (.+)
    • generic: Invalid character: 'S': failed to execute message is not the best, but does give some hint
  • Once I register a name, the success page says "Home". My ideal action afterwards would be to do to the details page of the same contract and the name I just registered, showing "You own 'name-just-registered'"
  • If I go to any page besides home, eg. http://localhost:3000/contract/free/cosmos1vjecguu37pmd577339wrdp208ddzymkudc46zj and then reload the app, it will give me an error "TypeError: accountProvider.account is undefined". I would expect a deep link to work as follows:
    • If I hit any page and the app is not initialized (some wrapper container)
    • Redirect me to the home page ("Browser/Keplr")
    • Provide some context to the home page, so once I hit "browser" I end up at the page I originally wanted (eg. free contract, not the contract list)
  • Check warning messages on console (searching names):
    • "Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Wave which is inside StrictMode."
    • "Warning: isInitialValid has been deprecated and will be removed in future versions of Formik. Please use initialErrors or validateOnMount instead."
abefernan commented

"Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Wave which is inside StrictMode."

Regarding this, it seems to be Ant Design's fault. It also affects Material UI, but they will fix it in version 5. I tried upgrading AntD but still shows that warning, I'll see if there is a workaround

abefernan commented

Once I register a name, the success page says "Home". My ideal action afterwards would be to do to the details page of the same contract and the name I just registered, showing "You own 'name-just-registered'"

I'm guessing we would want the same for transfer? Name detail that shows 'your-old-name' is owned by [other-address]? Also, what would be the ideal button text instead of "Home"?

ethanfrey commented

Regarding this, it seems to be Ant Design's fault. It also affects Material UI, but they will fix it in version 5. I tried upgrading AntD but still shows that warning, I'll see if there is a workaround

No problem. If is not our code and there is no published fix, don't worry about it.

ethanfrey commented

I'm guessing we would want the same for transfer? Name detail that shows 'your-old-name' is owned by [other-address]? Also, what would be the ideal button text instead of "Home"?

Yes

abefernan commented

You could search for "execute wasm contract failed: (.+); message index: (\d+)" and then display the (.+)

It seems that string is not always present:

image

So I just picked the last line but without the "message index" part.

  • 👍1
abefernan commented

Name validation doesn't check for lower case (try searching for SHELL and registering it, you get an error) - maybe better to fix after fixing the error display

The yup "lowercase" requirement makes it automatically transform to lowercase, without showing requirement error, but it doesn't seem to be compatible with formik, so I just apply a lowercase transformation before the useState setter.

ethanfrey commented

It seems that string is not always present:

Good check. Thanks

ethanfrey commented

I just apply a lowercase transformation before the useState setter.

Even better idea. Make it all lowercase without showing the users errors

abefernan commented

I would expect a deep link to work as follows [...]

I improved the whole routes, sdk & account init and state keeping and I think everything works, let me know if you find a case that I missed.