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

feat(twap-orders): twap orders deadline updates (8) #2460

Conversation

nenadV91
Copy link
Contributor

@nenadV91 nenadV91 commented May 15, 2023

Summary

  • Lots of code refactoring
  • Updates the deadline selector
  • Adds display for single interval time
  • Adds custom selector for TWAP orders
  • Adds hooks to parse predefined intervals or custom intervals to single interval time

Custom deadline selector
Screenshot 2023-05-15 at 16 52 54

Single interval time
Screenshot 2023-05-16 at 16 40 48

@vercel
Copy link

vercel bot commented May 15, 2023

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

Name Status Preview Comments Updated (UTC)
swap-dev 🔄 Building (Inspect) Visit Preview

🌃 Cosmos ↗︎

@socket-security
Copy link

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
@types/[email protected] None +0 types

@nenadV91 nenadV91 marked this pull request as ready for review May 16, 2023 14:49
@nenadV91 nenadV91 requested review from a team May 16, 2023 14:49
@nenadV91 nenadV91 added the TWAP label May 16, 2023
@elena-zh
Copy link
Contributor

Hey @nenadV91 , could you please provide a bit more information what should and what I should NOT test here?

@nenadV91
Copy link
Contributor Author

@elena-zh I would just test the functionality in general and if you think something is not working properly let me know.

@elena-zh
Copy link
Contributor

elena-zh commented May 17, 2023

@elena-zh I would just test the functionality in general and if you think something is not working properly let me know.

Got it, thanks.
However, it would be great to clarify a bit about areas to test.
Not fond of an idea of testing something blindly, especially when I get answers that issues I report are not a part of this PR, this or that is not yet implemented, etc.
As an example of a great PR description is in #2462 ;)

@elena-zh
Copy link
Contributor

Hey @nenadV91 , here are issues that I have faced:

  1. Sometimes the tooltip overlaps the selector, so I can't press on the time in order to open the dropdown
    tooltup overlaps
  2. When a part is less than 1 minute, the Part every section shows dash. The same issue is when a part should be 1 minute and some seconds
    or when seconds
    part every when less than 1 min
  3. When I apply 0 minutes and 0 seconds, the dropdown shows 'Select time'. My proposal here is whether:
  • not allow to set 0h and 0m,
  • OR show a default value if 0h 0m is selected
    0 0 - return to defaut
  1. Can we convert to days/hours/minutes when we select big numbers of hours and minutes (like it is done for Part every field)?
    convert to days like on parts
  2. Can we keep the tooltip closer to the label in this case?
    tooltip closer
  3. I can copy and paste a decimal value into the field, and the value will be saved. However, pre part value will be not correct:
    10 min
    paste

Thank you!

@nenadV91 nenadV91 changed the title feat(twap-orders): twap orders deadline updates feat(twap-orders): twap orders deadline updates (8) May 18, 2023
@nenadV91
Copy link
Contributor Author

@elena-zh

  1. But the tooltip is removed when you move the mouse right?
  2. Fixed, I've added seconds
  3. Fixed, it will be disabled
  4. Fixed
  5. Fixed I think, the label wont break line now
  6. Fixed, dots will be removed now on paste

* feat: refactor twap orders code

* feat: some small updates

* fix: pr comments 1

* fix: pr comments 2
@nenadV91 nenadV91 merged commit 95bfffb into feature/twap-orders-parts-display May 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2023
@elena-zh
Copy link
Contributor

Hey @nenadV91 , I have caught the app crash when I opened the Advanced orders tab
image

@alfetopito alfetopito deleted the feature/twap-orders-deadline-updates branch May 23, 2023 09:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants