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: error handling ux improvements #162

Closed
wants to merge 8 commits into from
Closed

Conversation

denviljclarke
Copy link
Contributor

@denviljclarke denviljclarke commented Nov 22, 2024

Description

The PR adds some validation and improved messaging. I just added an error message for missing quote in the case of no other error. We could refactor to get the actual error but more often than not missing quote errors are because missing rates so I've just added a message to reflect this

Other changes

Increased the refetch interval of the swap quote by two seconds.
Add debouncing to validation logic to prevent excessive SDK calls and heavy computations on each keystroke

Tested

Swapped and tried to trigger errors on the preview deployment

Related issues

  • Fixes

Checklist before requesting a review

Copy link

vercel bot commented Nov 22, 2024

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

Name Status Preview Comments Updated (UTC)
mento-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 5:36pm

@Andrew718PLTS
Copy link
Contributor

Andrew718PLTS commented Dec 4, 2024

@nvtaveras Here are some issue found during testing:

  1. Doesn't handle a case when no wallet connected and put a large number (e.g from the bug ticket: 7478773923223123234925). I think it should not change state to the grey continue button and after to the red with connect wallet text. One of them would be nice:
    (I left a short video below due to GitHub restrictions)
  2. It looks like validation with the grey continue button state after making some actions works fine BUT, I can't get my loadded balances for tokens after wallet connection - do you have this opportunity on vercel link?
    image

@Andrew718PLTS
Copy link
Contributor

Screen.Recording.2024-12-04.at.12.50.04.mov

@Andrew718PLTS
Copy link
Contributor

image

  1. Still can proceed with a larger amount than balance (reproduced with CELO that equals 20 with 747877392322312323492566622277023 amount).
  2. It breaks the page on the Confirm Swap state

@Andrew718PLTS
Copy link
Contributor

Andrew718PLTS commented Dec 27, 2024

Bug-fix list related to this ticket:

  • Doesn't handle a case when no wallet is connected and puts a large number (e.g., from the bug ticket: 7478773923223123234925)
  • Incorrect calculation of a larger amount than the balance with more than 22 digits and proceeding to Confirm state
  • The Confirm Swap page is corrupted when there are more than 22 numbers
  • The "Balance still loading" state for a button is stuck when any amount field is not changing (applied dynamical button on balance loading)
  • Missing specified toast errors on too large amounts
  • 'Trading temporarily paused' handled for 'no valid median' and 'trading suspended' cases and moved to the toast message

Copy link
Contributor

@RyRy79261 RyRy79261 left a comment

Choose a reason for hiding this comment

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

Couldn't find functional faults, I would prefer the truncation just done with CSS & the variations managed by CVA but this should be a more uniform change if done.

Within scope, this PR seems fine

{ fromTokenSymbol, toTokenSymbol }: IGetToastErrorOptions = {}
): string {
switch (true) {
case swapErrorMessage.includes(`overflow x1y1`):
Copy link
Contributor

@nvtaveras nvtaveras Jan 9, 2025

Choose a reason for hiding this comment

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

What's the full error message that come from this error? Wondering if there's anything better we could use. Is it different from the one below, can't create fixidity number larger than

Copy link
Contributor

Choose a reason for hiding this comment

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

Here are the complete error messages regarding entering an too large number in the fields:
swap-out: overflow x1y1 * fixed1 detected
swap-in: can't create fixidity number larger than maxNewFixed()

@nvtaveras
Copy link
Contributor

@Andrew718PLTS once I enter a large number 7478773923223123234925 without a wallet connected, it takes ~5 seconds to show the error message saying that the amount is too large. Why is that the case?

@Andrew718PLTS
Copy link
Contributor

@Andrew718PLTS once I enter a large number 7478773923223123234925 without a wallet connected, it takes ~5 seconds to show the error message saying that the amount is too large. Why is that the case?

Previously, the calculation has not been correctly working when entering >=22 (e.g., 7478773923223123234925 -> 7.479)
It works the same way when a wallet is connected. If the getAmountOut or getAmountIn methods throw an error with the declared message, it shows an error toast with the specified message, depending on the method's error message.

@Andrew718PLTS Andrew718PLTS changed the title feat: add validation & error handling ux improvements feat: error handling ux improvements Jan 13, 2025
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.

4 participants