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: [GSW-1975] SwapState Loading & Insufficient Liquidity #587

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

tfrg
Copy link
Collaborator

@tfrg tfrg commented Dec 10, 2024

Description

  • Improve handling of insufficient liquidity state in Swap page
  • added the loading state to improve the UI

Details

  • Improve swap liquidity limit detection and handling in the swap feature
  • Implement proper liquidity threshold tracking for better user experience
    -Implement estimatedLiquidityMax-based fetch control

Copy link

vercel bot commented Dec 10, 2024

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

Name Status Preview Comments Updated (UTC)
gnoswap-interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 23, 2024 8:00am

@tfrg tfrg requested a review from roje1 December 10, 2024 07:47
@tfrg tfrg added the DO NOT MERGE Required to check merge timing label Dec 10, 2024
notJoon
notJoon previously approved these changes Dec 13, 2024
tfrg added 15 commits December 23, 2024 10:22
Refactor this getter to return a value.
Getters and setters should access the expected fields typescript:S4275
Software qualities impacted:
Reliability
Provide a compare function to avoid sorting elements alphabetically.
"Array.prototype.sort()" and "Array.prototype.toSorted()" should use a compare function typescript:S2871
Software qualities impacted:
Reliability
Fix SonarQube issue by removing direct .sort() comparison in test assertions
Remove this duplicated type or replace with another one.
Union and intersection types should not include duplicated constituents typescript:S4621
Software qualities impacted:
Maintainability
Refactor this redundant 'await' on a non-promise.
"await" should only be used with promises typescript:S4123
Software qualities impacted:
Maintainability
Move this "default" clause to the end of this "switch" statement.
"default" clauses should be last typescript:S4524
Software qualities impacted:
Maintainability
Refactor this function to reduce its Cognitive Complexity from 60 to the 15 allowed.
Cognitive Complexity of functions should not be too high typescript:S3776
Software qualities impacted:
Maintainability
Refactor this function to reduce its Cognitive Complexity from 60 to the 15 allowed.
Cognitive Complexity of functions should not be too high typescript:S3776
Software qualities impacted:
Maintainability
Refactor this function to reduce its Cognitive Complexity from 60 to the 15 allowed.
Cognitive Complexity of functions should not be too high typescript:S3776
Software qualities impacted:
Maintainability
Make sure not using "noopener" is safe here.
Authorizing an opened window to access back to the originating window is security-sensitivetypescript:S5148
… operator

Extract this nested ternary operation into an independent statement.
Ternary operators should not be nested typescript:S3358
Software qualities impacted:
Maintainability
Copy link
Member

@notJoon notJoon left a comment

Choose a reason for hiding this comment

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

LGTM. left some comments

packages/web/src/hooks/swap/data/use-swap.tsx Outdated Show resolved Hide resolved
packages/web/src/hooks/swap/data/use-swap.tsx Outdated Show resolved Hide resolved
packages/web/src/hooks/swap/data/use-swap.tsx Show resolved Hide resolved
packages/web/src/utils/pool-utils.ts Outdated Show resolved Hide resolved
- Simplify array concatenation using spread operator and join
- Keep null/zero amount check for preventing unnecessary API calls
- Maintain proper handling of decimal input cases (0 -> 0.1 -> 0.12)
Copy link

Copy link
Member

@notJoon notJoon left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Required to check merge timing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants