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

UI for proof test with cid #149

Merged
merged 21 commits into from
Jan 7, 2022
Merged

UI for proof test with cid #149

merged 21 commits into from
Jan 7, 2022

Conversation

Tbaut
Copy link
Contributor

@Tbaut Tbaut commented Jan 6, 2022

This is a simple UI with relatively helpful messages if:

  • the api can't connect to the node, e.g the node isn't started
  • the user doesn't have the polkadot extension installed, or if they have no account
  • if the node throws error at the proof verification, we display them (see demo below)

For now the wallet account is useless since there is nothing that needs to be signed.

test.mp4

@Tbaut Tbaut requested review from mattsse, clearloop and willeslau and removed request for mattsse January 6, 2022 22:33
Copy link
Contributor

@clearloop clearloop left a comment

Choose a reason for hiding this comment

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

lgtm! tested on my local machine!

btw should we add /ui in /package.json?

@clearloop clearloop added the Status: Approved Added to a PR when the required number of approvals have been met. label Jan 7, 2022
Copy link
Contributor

@mattsse mattsse 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 great!

@Tbaut
Copy link
Contributor Author

Tbaut commented Jan 7, 2022

@clearloop what is lerna used for? Did you set it up so that you can use any package locally without needing them to be published?
I'm happy to explore this in another PR, not sure how much it would break :D

@clearloop
Copy link
Contributor

clearloop commented Jan 7, 2022

Did you set it up so that you can use any package locally without needing them to be published?

yarn workspace already supported this feature, once you made /ui a member of /package.json, you can build @chiansafe/filecoindot-types locally but not from the registry

https://classic.yarnpkg.com/lang/en/docs/workspaces/#toc-how-does-it-compare-to-lerna

currently lerna is just for the shortcut of running yarn build for each packages in /js, as a preprocess for publishing packages to the npm registry

filecoindot/package.json

Lines 15 to 20 in 7efe143

"scripts": {
"build": "yarn && lerna run build",
"publish": "lerna run publish",
"e2e": "yarn && ts-node js/e2e/index.ts",
"setup": "ts-node e2e/setup.ts"
},

also, we can use lerna to do the version control stuff for our javascript packages in the future

I'm happy to explore this in another PR, not sure how much it would break :D

mb you can try to play with the version control stuff with lerna LOL, it's really a tough topic in javascript package management I think, and lerna is also hard to control in my past experience...

and for the metadata part, you were right, we don't need to write types for filecoindot since metadata-v14 handled it perfectly, we haven't tag #[metadata] to the rust types before and it was required when I wrote the types, but since we had upgraded paritytech/substrate#8615, the #[metadata] tags are no more required and we just can get the rust types from the rust rpc....but, still need to declare the rpc types

@clearloop clearloop merged commit d0c6d87 into main Jan 7, 2022
@clearloop clearloop deleted the web-example branch January 7, 2022 12:02
@Tbaut
Copy link
Contributor Author

Tbaut commented Jan 7, 2022

yarn workspace is what I've seen used the most indeed, and ppl, just like you mentioned were avoiding lerna as much as possible.
I won't play with yarn workspace immediately, because things work so far, but I'll keep this in mind for the future eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Approved Added to a PR when the required number of approvals have been met.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants