-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactor dropdown logic #2506
Refactor dropdown logic #2506
Conversation
09e5544
to
3988121
Compare
|
||
const compilerContracts = this.dropdownLogic.getCompilerContracts() | ||
const confirmationCb = this.getConfirmationCb(modalDialog, confirmDialog) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you have only one source of error possible I would recommend :
try {
const contractMetadata = await this.runView.call('compilerMetadata', 'deployMetadataOf', selectedContract.name)
const compilerContracts = this.dropdownLogic.getCompilerContracts()
const confirmationCb = this.getConfirmationCb(modalDialog, confirmDialog)
} catch (error) {
return statusCb(`creation of ${selectedContract.name} errored: ` + error)
}
if (!content.gasPriceStatus) { | ||
cancelCb('Given gas price is not correct') | ||
} else { | ||
var gasPrice = this.dropdownLogic.toWei(content.querySelector('#gasprice').value, 'gwei') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use const
when adding new piece of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do the whole syntax update stuff on the remix-side of things in a different PR (just like for remix)
determineGasFees (tx) { | ||
const determineGasFeesCb = (gasPrice, cb) => { | ||
let txFeeText, priceStatus | ||
// TODO: this try catch feels like an anti pattern, can/should be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create an issue for that and reference it here: TODO(#1234) ...
?
(finally) extracts the modal/confirm dialogs from the dropdown logic, moves logic related to execution context and udapp into a blockchain module