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.

Sign up for GitHub

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

Jump to bottom

feat: limit display delta between price and market #8412

Merged
merged 2 commits into from
Dec 19, 2024
Merged

feat: limit display delta between price and market #8412

merged 2 commits into from
Dec 19, 2024

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 19, 2024 •
edited
Loading

Description

Does what it says on the box.

Issue (if applicable)

closes #8410

Risk

High Risk PRs Require 2 approvals

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

Low to none

Testing

  • Market shouldn't display delta
  • Loading quote shouldn't display delta, regardless of the percentage option you have previously chosen
  • Negative delta should be displayed as percentage in red
  • Positive delta should be displayed as percentage in red
  • Removing limit price altogether (empty string) should not display delta
  • Inverted limit direction should work the exact same

Engineering

  • ^

Operations

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

Screenshots (if applicable)

  • Regular direction
Screenshot 2024-12-19 at 13 41 13 Screenshot 2024-12-19 at 13 41 01 Screenshot 2024-12-19 at 13 40 56
  • Inverted direction
Screenshot 2024-12-19 at 13 41 26 Screenshot 2024-12-19 at 13 41 34 Screenshot 2024-12-19 at 13 41 34
  • Empty string input
Screenshot 2024-12-19 at 13 44 18

@gomesalexandre gomesalexandre requested a review from a team as a code owner December 19, 2024 10:47
NeOMakinG
NeOMakinG approved these changes Dec 19, 2024
View reviewed changes
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.

Considering we have a follow up to fix the Inifinity, NaN bugs, we can get this one in

https://jam.dev/c/eb938b78-1a2a-439a-8015-860fbfa039f5

Also, we might want to take a look to the case where the % is very very big

@NeOMakinG NeOMakinG merged commit fe2369d into develop Dec 19, 2024
3 checks passed
@NeOMakinG NeOMakinG deleted the fix_limit_display_delta branch December 19, 2024 13:53
@gomesalexandre gomesalexandre mentioned this pull request Dec 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NeOMakinG NeOMakinG NeOMakinG approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Limit orders - Display delta between selected price and market
2 participants
@gomesalexandre @NeOMakinG