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

add form shortcuts, min vol input #1944

Merged
merged 6 commits into from Sep 4, 2022
Merged

add form shortcuts, min vol input #1944

merged 6 commits into from Sep 4, 2022

Conversation

smk762
Copy link
Collaborator

@smk762 smk762 commented Aug 24, 2022

closes: #1941
closes: #1459

orderform_update.mp4

@smk762 smk762 requested review from a user, tonymorony, cipig, SirSevenG and Canialon August 24, 2022 05:07
SirSevenG
SirSevenG previously approved these changes Aug 25, 2022
@tonymorony
Copy link
Contributor

Thats a great feature, thank you!

  1. I've noticed that inaccuracy accumulating when I press +1% (I assume it should be around 8% when I click 8 times but it's 8.32% what is quite huge diff)
Screen.Recording.2022-08-25.at.14.45.14.mov
  1. For a new user it might be not clear what -1% 0% 1% buttons means, it would be awesome to add a tooltip with clarification

Copy link
Contributor

@tonymorony tonymorony left a comment

Choose a reason for hiding this comment

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

^^ it worth to double-check calculations, seems to be not precise

@smk762
Copy link
Collaborator Author

smk762 commented Aug 25, 2022

^^ it worth to double-check calculations, seems to be not precise

At the moment, the +1% is relative to currently input price, so it is compounding.
If you think its better to make it a consistent 1% relative to market, I can change that easy enough.

The only problem with this is if the coin has no cex price to calculate from. In this case, I can revert to compounding relative to price input field.

@tonymorony
Copy link
Contributor

^^ it worth to double-check calculations, seems to be not precise

At the moment, the +1% is relative to currently input price, so it is compounding. If you think its better to make it a consistent 1% relative to market, I can change that easy enough.

The only problem with this is if the coin has no cex price to calculate from. In this case, I can revert to compounding relative to price input field.

It depends on how these buttons should act:

0% button now setting price same as CEX, if -1% and +1% operating with price in field it's a bit inconsistent/confusing imo (at least without additional clarification for the user, e.g. I've expected that -1% +1% setting price 1% better or worse vs CEX price)

@smk762
Copy link
Collaborator Author

smk762 commented Aug 25, 2022

non-compounding is more intuitive, tho with tooltip explaining the compounding way for both priced and unpriced would be more consistent (otherwise long tooltip with conditionals).

Another alternative could be to remove those buttons for unpriced pairs (right now clicking the zero on an unpriced coin is pointless)

@tonymorony
Copy link
Contributor

non-compounding is more intuitive, tho with tooltip explaining the compounding way for both priced and unpriced would be more consistent (otherwise long tooltip with conditionals).

Another alternative could be to remove those buttons for unpriced pairs (right now clicking the zero on an unpriced coin is pointless)

maybe we should use both? (don't show buttons for assets without CEX price and in case if they showing show a ? tooltip with explanation

tonymorony
tonymorony previously approved these changes Aug 29, 2022
Copy link
Contributor

@Canialon Canialon left a comment

Choose a reason for hiding this comment

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

@smk762 this tip is a good idea, but they covered the field with information. So to see how the amount changed in this field I need to move out the mouse every time. Can you please replace the tips on the bottom of the buttons? I hope it will be better

image


I can set a negative price if I enter 0 and click the -1 button
image
Anyway, this swap will fail because of minimum price( [{"error":"rpc:212] dispatcher_legacy:160] lp_ordermatch:3565] lp_ordermatch:3646] Price is too low, minimum is 0.00000001"}] ). But I guess it would be good to eliminate the possibility of setting a negative value

Canialon
Canialon previously approved these changes Aug 30, 2022
@tonymorony
Copy link
Contributor

found a bug I think:
I can see "Min volume" I've input when I deactivate "Use custom minimum trade amount" - I assume min trade volume shouldn't apply if deactivated:

Screen.Recording.2022-08-31.at.17.45.24.mov

@tonymorony tonymorony self-requested a review August 31, 2022 15:47
Copy link
Contributor

@tonymorony tonymorony left a comment

Choose a reason for hiding this comment

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

would be great to fix ^^

@smk762
Copy link
Collaborator Author

smk762 commented Sep 1, 2022

I've also made the form stop bouncing around when you toggle the min volume field, get an error message or the "Order selected" box is displayed. Previously, when these became visible, it would shift other form elements - now everything should stay where it is when these things change between visible or hidden.

@tonymorony tonymorony merged commit e576625 into dev Sep 4, 2022
@smk762 smk762 mentioned this pull request Nov 18, 2022
@smk762 smk762 deleted the orderform_shortcuts branch August 7, 2023 07:42
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.

[FEATURE REQUEST]: Add an "at market" button to pro view form field for custom minimum trade amount
4 participants