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

GasEditor component #3750

Merged
merged 3 commits into from Dec 9, 2016
Merged

GasEditor component #3750

merged 3 commits into from Dec 9, 2016

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Dec 8, 2016

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 8, 2016
@observable errorEstimated = null;
@observable errorGas = null;
@observable errorPrice = null;
@observable estimated = DEFAULT_GAS;
Copy link
Contributor

@derhuerst derhuerst Dec 8, 2016

Choose a reason for hiding this comment

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

Now that the gas price calculations are fully externalised, I'd prefer to have numbers here, not strings. Same for setGas and setPrice.

Copy link
Contributor Author

@jacogr jacogr Dec 8, 2016

Choose a reason for hiding this comment

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

Feel free to log an issue if it is critical and put it in the queue. It is not hapenning in this PR. Bear in mind -

  1. Everything BigNumbers at the very least
  2. Data coming from inputs are strings as it stands, with a mixture coming in via BigNumber (typically RPC BN -> input text -> BN -> ? would need to be done)
  3. Impact on other stores (eg. Transfer that expects things in certain formats)

@derhuerst derhuerst added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 9, 2016
@jacogr jacogr merged commit 2582514 into master Dec 9, 2016
@jacogr jacogr deleted the jg-gaseditor branch December 9, 2016 12:44
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.

None yet

2 participants