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

Market bug fixes #728

Merged
merged 5 commits into from
Nov 30, 2016
Merged

Market bug fixes #728

merged 5 commits into from
Nov 30, 2016

Conversation

svk31
Copy link
Contributor

@svk31 svk31 commented Nov 29, 2016

Also includes an update to the README for Ubuntu 16.x

@roadscape
Copy link
Contributor

Awesome, just 2 notes:

  1. With regards to the MySQL note, is it necessary to use root or could we include instructions for creating something like a steem user if it's just as straightforward? (I'm assuming root does not work like it used to for enhanced default security out of the box?)
  2. buy_price_warning and sell_price_warning were originally used to warn the user if the "effective" rate differed from the "entered" rate by more than 1%. This can happen when the amounts involved are small -- for example:
    image
    (The entered rate is 0.010 SD/STEEM, but the effective rate is 0.00666.... The "Price" field becomes grayed as a cheap way to alert the user.)
    Overall, since this 'quirk' only applies to small amounts, it's of relatively low importance. And although it did cause enough confusion to be reported, the case you repurposed these props for will be useful for more users and greater amounts.
    If there is a simple way to warn the user in these cases too it would be nice to have; otherwise, let's leave it as-is.

@valzav
Copy link
Contributor

valzav commented Nov 29, 2016

@roadscape could you please review?

@svk31
Copy link
Contributor Author

svk31 commented Nov 29, 2016

For the mysql, that's beyond me tbh, I just found those instructions on stackoverflow as other users have reported similar issues. I think that the mysql root user should be independent from your OS root user, but for some reason that's not the case in Ubuntu 16.x.

About the percentDiff method, it must've gotten bugged at one point because it was always returning 0 since the two amounts input were always the same.

The greying out still happens, but now whenever the price differs by 15% or more from the market prices. The 15% is completely open to tuning, it's just a number that seemed ok to me.

@roadscape
Copy link
Contributor

About the percentDiff method, it must've gotten bugged at one point because it was always returning 0 since the two amounts input were always the same.

It would probably return 0 for most values which are not in the sub-10 satoshi ranges. It does seem better to repurpose it for this case; 15% seems reasonable. I'll give it a final test and merge today.

@jcalfee
Copy link
Contributor

jcalfee commented Nov 30, 2016

Great, thanks for the mysql fix. That is tricky.

In the future we should probably use a bound component <input value={this.state.buySteem_price} instead of setting the value directly. I'll just assume that was a more invasive change.
componentWillReceiveProps() { ... if (this.refs.buySteem_price) this.refs.buySteem_price.value = parseFloat(lowest_ask).toFixed(6); ... }

.. or correct me if I'm missing something. Thanks..

@svk31
Copy link
Contributor Author

svk31 commented Nov 30, 2016

Yes the value change via refs was just to stay in line with how it was already being done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants