-
Notifications
You must be signed in to change notification settings - Fork 3
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
[FIX][STK-252] - Update My Stack tabs after stack cancellation #209
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -231,6 +231,7 @@ export const StackOrders = ({ chainId, address }: StackOrdersProps) => { | |||
stacks.orders, | |||
stacks.sort | |||
)} | |||
refetchAllOrders={fetchAllOrders} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why no use fetchAllOrders
instead of refetchAllOrders
?
cause it's seems to fetchAllOrders and not necessarily has the logic for re-fetching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a refetch, right? Also we are using refetchStacks with the same logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly as Leo suggested, the idea for this prop is that, when we successfully cancel a stack and close the modal, we re-fetch the needed data:
- The total orders count used for the tabs count (and the "All stacks count")
- The displayable orders per page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but we're passing it a fetchAllOrders
function, I would keep the same name. As the function fetches the orders. Where we use it is the re-fetch thing.
I would keep the same name but okay with whatever you guys think its the best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I get the semantic you suggest now, I'm ok with your suggestion, will push it in a bit.
Deploying stackly-ui with Cloudflare Pages
|
Fixes: STK-252
Description
stack-order.ts
to cover an edge case where it might break the app on a fast tab switch, to get the remaining funds of the stack.Visual Evidence
https://www.loom.com/share/3dbdab016ac840b09b77f8e63076ced0?sid=d5e43b35-a395-4e2b-b6ae-9f3c3288924f