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(twap): refactor and wire price protection #2797

Merged
merged 8 commits into from
Jul 6, 2023
Merged

feat(twap): refactor and wire price protection #2797

merged 8 commits into from
Jul 6, 2023

Conversation

alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Jul 5, 2023 •
edited
Loading

Summary

On top of #2785

Screen.Recording.2023-07-05.at.17.01.31.mov

To Test

  1. On TWAP tab, fill in order params
  • Price protection should show the price minus slippage
  • Clicking on price row should invert both prices

@vercel
Copy link

vercel bot commented Jul 5, 2023 •
edited by github-actions bot
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback

🌃 Cosmos ↗︎

@alfetopito alfetopito self-assigned this Jul 5, 2023
@alfetopito alfetopito added the TWAP label Jul 5, 2023
@alfetopito alfetopito requested review from a team July 5, 2023 15:59
@alfetopito alfetopito mentioned this pull request Jul 5, 2023
Merged
@alfetopito alfetopito changed the title Twap form styling 1 wire logic feat(twap): refactor and wire price protection Jul 5, 2023
elena-zh
elena-zh requested changes Jul 6, 2023
View reviewed changes
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Hey @alfetopito , nice, thanks!
However, I don't see an amount when I'm not connected to a wallet.
image

Then, when we have 0 as an amount, maybe it is better to show 0 instead of the dash as it makes an impression that a value is a negative one?
0

Could you please take a look at these issues?

@alfetopito
Copy link
Collaborator Author

Hey @alfetopito , nice, thanks! However, I don't see an amount when I'm not connected to a wallet. image

Then, when we have 0 as an amount, maybe it is better to show 0 instead of the dash as it makes an impression that a value is a negative one? 0

Could you please take a look at these issues?

Both addressed

@alfetopito alfetopito requested a review from elena-zh July 6, 2023 13:32
elena-zh
elena-zh approved these changes Jul 6, 2023
View reviewed changes
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

LGTM

@alfetopito alfetopito merged commit a657074 into twap-form-styling-1 Jul 6, 2023
12 checks passed
@alfetopito alfetopito deleted the twap-form-styling-1_wire-logic branch July 6, 2023 14:04
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2023
anxolin
anxolin reviewed Jul 7, 2023
View reviewed changes
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

👍

not from this PR, but the style of the percentage should allow a few decimals.
nit: Also, the default should be 10% not 10.0% right?

image

Also, this is breaking if i add for example 10.12 in the slippage
image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anxolin anxolin anxolin left review comments

@shoom3301 shoom3301 shoom3301 approved these changes

@elena-zh elena-zh elena-zh approved these changes

Assignees

@alfetopito alfetopito

Labels
TWAP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alfetopito @anxolin @shoom3301 @elena-zh