Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fixes to the Token Registration dApp #2250

Merged
merged 18 commits into from
Sep 23, 2016
Merged

Fixes to the Token Registration dApp #2250

merged 18 commits into from
Sep 23, 2016

Conversation

ngotchac
Copy link
Contributor

Small fixes for issues #2195, #2198, #2193, #2197, #2196, #2194 ...

@parity-cla-bot
Copy link

It looks like this contributor signed our Contributor License Agreement. 👍

Many thanks,

Ethcore CLA Bot

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M-ui labels Sep 22, 2016
if (this.state.loading || !this.state.valid) return;

return (
<div className={ styles['input-icon'] }>
Copy link
Contributor

@jacogr jacogr Sep 22, 2016

Choose a reason for hiding this comment

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

I believe styles.inputIcon may do the trick as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, no...


this.setState({ ...validation });

if (validation.valid) return this.props.onChange(true, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure the linter doesn't complain here? (Single-line if)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it doesn't, but modified it.

@@ -185,13 +276,14 @@ export default class Token extends Component {
}

onMetaLookup = () => {
let query = this.state.metaQuery;
let keyIndex = this.state.metaKeyIndex;
Copy link
Contributor

Choose a reason for hiding this comment

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

let vs const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I always use const?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Unless we need to modify things. The best approach - just use const, if you assign something and it complains, let it :)

})
.then(data => {
// If no total supply, must not be a proper token
if (data.totalSupply === null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

does it come back at null or 0? May want to do a eth_getCode on the address first, no code found, not a contract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, the getTokenTotalSupply method is in fact doing just that, returning null if no code, or not a token contract.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 23, 2016
@jacogr jacogr merged commit cfbb0e7 into js Sep 23, 2016
@jacogr jacogr deleted the ng-tokenreg-ui-fixes branch September 23, 2016 16:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants