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: use actual input output difference #7817

Merged
merged 3 commits into from
Sep 26, 2024
Merged

fix: use actual input output difference #7817

merged 3 commits into from
Sep 26, 2024

Conversation

0xApotheosis
Copy link
Contributor

Description

Updates the value shown in the "buy amount fiat difference" part of the trade input to actually show the percentage difference in the fiat amount received vs the fiat amount sent (excluding gas).

Previously, this was showing the price impact, which did not match the copy.

Issue (if applicable)

Closes #7790

Risk

High Risk PRs Require 2 approvals

Small

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

None

Testing

  • Ensure the input/output percentage looks correct for a range of trades with and without fees.
  • Ensure the correct color-coding is used: white if less than 1%, yellow if less than 3%, red if greater than 3%. Green if positive!

Engineering

☝️

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

☝️

Screenshots (if applicable)

Screenshot 2024-09-26 at 12 33 28 Screenshot 2024-09-26 at 12 37 55 Screenshot 2024-09-26 at 12 38 38

@0xApotheosis 0xApotheosis requested a review from a team as a code owner September 26, 2024 02:39
@0xApotheosis 0xApotheosis changed the title chore: improve translation key name fix: use actual input output difference Sep 26, 2024
@NeOMakinG NeOMakinG self-requested a review September 26, 2024 07:25
Copy link
Collaborator

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

Very small comment, pending runtime test

Looks good overall, the logic seems straightforward

Copy link
Collaborator

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

image
image
image
image
image

All good! ✅

@NeOMakinG NeOMakinG enabled auto-merge (squash) September 26, 2024 10:30
@NeOMakinG NeOMakinG merged commit 9c73047 into develop Sep 26, 2024
3 checks passed
@NeOMakinG NeOMakinG deleted the price-impace-ui branch September 26, 2024 10:37
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.

Swapper price impact calculation inaccurate
3 participants