Skip to content

Conversation

@DanielSchiavini
Copy link
Collaborator

@DanielSchiavini DanielSchiavini commented Dec 12, 2025

  • fix the validation of the queries instead of passing booleans manually
  • some errors were being caused while the max debt was loaded
  • add some throttling to the form values sent back to the market page, as they cause a lot of store calls

@vercel
Copy link

vercel bot commented Dec 12, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
curve-dapp Ready Ready Preview, Comment Dec 16, 2025 8:29am
curve-dapp-storybook Ready Ready Preview, Comment Dec 16, 2025 8:29am

@DanielSchiavini DanielSchiavini marked this pull request as ready for review December 12, 2025 15:54
@OnlyJousting
Copy link
Contributor

OnlyJousting commented Dec 15, 2025

When simulating a max loan on "high ltv" and switching over to "safe" there is no error being shown in the form about the debt being too high. The value in "max borrow" is also not possible to click to update the "borrow" input

Screen.Recording.2025-12-15.at.5.43.11.mov

I was able to initiate a transaction with an incorrect "borrow" value in the input

SCR-20251215-ftln

@OnlyJousting
Copy link
Contributor

This branch also has broken styles for the action form sub nav and the self-liquidate section but it is probably unrelated to this PR

@DanielSchiavini
Copy link
Collaborator Author

DanielSchiavini commented Dec 15, 2025

When simulating a max loan on "high ltv" and switching over to "safe" there is no error being shown in the form about the debt being too high. The value in "max borrow" is also not possible to click to update the "borrow" input

Thank you, that was a good point. Apparently the vest form only validates fields that change, so if the debt is changed, we need to revalidate the maxDebt and vice-versa 😞

The fix is in 987d1e0 but I'd like to add some tests

This branch also has broken styles for the action form sub nav and the self-liquidate section but it is probably unrelated to this PR

Yes I've seen this before, indeed. I fixed it in one of the follow-up PRs:

@OnlyJousting
Copy link
Contributor

Now the "debt too high" error remains after updating the "borrow" value to "max":

Screen.Recording.2025-12-15.at.19.44.29.mov

@DanielSchiavini
Copy link
Collaborator Author

Now the "debt too high" error remains after updating the "borrow" value to "max":

I believe you checked an outdated version, the test I introduced does exactly this and it's passing

* This is necessary because passing UseQueryResult to any react component will crash the rendering due to
* react trying to serialize the react-query proxy object.
*/
const q = <T,>({ data, isLoading, error }: UseQueryResult<T>): Query<T> => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Souldn't we export it and place it in some utils to be reused elsewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'll move it as soon as it's used in the follow-up PR

sliderProps?: SliderInputProps<Decimal>['sliderProps']

/** Optional children to be rendered below the input */
children?: ReactNode
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should update the storybook maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, I'll do that in another PR

@DanielSchiavini DanielSchiavini merged commit d4b908d into main Dec 16, 2025
16 checks passed
@DanielSchiavini DanielSchiavini deleted the fix/loan-queries branch December 16, 2025 09:40
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.

5 participants