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

Fix sats text on range orders #1332

Merged
merged 2 commits into from
Jun 16, 2024

Conversation

KoalaSat
Copy link
Member

@KoalaSat KoalaSat commented Jun 16, 2024

What does this PR do?

When a order with a range was taken, it shows always the value in sats for the max range, regardless the amount chosen

Checklist before merging

  • Install pre-commit and initialize it: pip install pre-commit, then pre-commit install. Pre-commit installs git hooks that automatically check the codebase. If pre-commit fails when you commit your changes, please fix the problems it points out.

@@ -178,7 +178,7 @@ const OrderDetails = ({
: coordinator.info?.taker_fee ?? 0;
const defaultRoutingBudget = 0.001;
const btc_now = order.satoshis_now / 100000000;
const rate = order.amount > 0 ? order.amount / btc_now : Number(order.max_amount) / btc_now;
const rate = Number(order.max_amount ?? order.amount) / btc_now;
Copy link
Member Author

Choose a reason for hiding this comment

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

btc_now is calculated over max_amount if exists

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am assuming this has been tested. Based on the change of logic alone I am not sure what the result would be (when is max_amount null | undefined? no idea), but looks like this probably fixes the issue 😄 Merging as is!

@Reckless-Satoshi Reckless-Satoshi merged commit ba4b641 into RoboSats:main Jun 16, 2024
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

2 participants