-
Notifications
You must be signed in to change notification settings - Fork 101
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: warn about unsigned orders, and hide not relevant orders #5214
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
That would be fantastic, I'm waiting for it 2 years :) |
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.
Hey @anxolin , great job!
I have faced 1 issue, and will add some nitpicks:
-
(issue): Partially fillable orders (they are all technically cancelled or expired) are hidden by default. So I'd add a check if an order has at least 1 fill, then show it on the main list
-
(nit): I'd make this text center-aligned in a mobile resolution
-
(nit): on an order details, I'd make this message left-aligned (like all the rest fields on the UI)
-
(not sure) maybe move these button to the left on a mobile resolution, so a user does not scroll to the right to see these buttons?
-
(I can open a separate task for this) I think we can re-use these button's design for the filter :)
@elena-zh fixed For nitpics, I don't have capacity as im working on something else, if @shoom3301 or @alfetopito want to reiterate, go for it. Otherwise, lets just reiterate after. I have now a 1h called. I would love to release a hotfix after that. |
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.
Great work!
apps/explorer/src/components/orders/OrdersUserDetailsTable/index.tsx
Outdated
Show resolved
Hide resolved
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.
Great, thank you!
I've opened #5218 task for my nitpicks.
Co-authored-by: Leandro <[email protected]>
Late feedback, but I really like this feature. |
Summary
This PR tries to address several issues related to the explorer. Primarily its displaying a warning for unsigned orders, so users know that the owner can be faked and anyone could have placed the order. Also, it makes some improvements to make the order table result more relevant for users.
Explorer shows not relevant orders
Some accounts have a lot of expired and canceled orders. When you go over your history, these orders are usually not super relevant to you.
Because for now the API don't allow any filtering by the status of the order, I applied a filter in the client.
Test with
0x79063d9173C09887d536924E2F6eADbaBAc099f5
account: https://explorer-dev-git-hide-canceled-expired-cowswap.vercel.app/address/0x79063d9173C09887d536924E2F6eADbaBAc099f5You should only be able to see the relevant orders:
Notice the filter on top of the table. It allows you to show the hidden orders.
Toggle the filter to show canceled or expired orders:
Handle empty pages
This PR will also handle pages, where all orders are filtered client side.
You can test with
0x6810e776880c02933d47db1b9fc05908e5386b96
account. All orders are hidden: https://explorer-dev-git-hide-canceled-expired-cowswap.vercel.app/address/0x6810e776880c02933d47db1b9fc05908e5386b96But you can reveal the orders:
Hide pre-sign orders
Similarly to canceled or expired, if the page has at least one order in the state of
presign
, we will hide it.You can test
0x5be9a4959308A0D0c7bC0870E319314d8D957dBB
account. You can see how most of the orders are hidden, because from the 20 orders, only one is shown: https://explorer-dev-git-hide-canceled-expired-cowswap.vercel.app/address/0x5be9a4959308A0D0c7bC0870E319314d8D957dBB10 are canceled or expired, and we can see them as I showed before. 9 are unsigned.
If you show the unsigned, it will show a warning to make sure users don't trust the owner:
Show warning for the details
The warning to not trust the owner until an order is signed is added to the details to.
Try to open
0xe16f5e497eed1a90d570542a365ad0791e42a23f1960acac4274312ba64e707a5be9a4959308a0d0c7bc0870e319314d8d957dbb6769acd3
order: https://explorer-dev-git-hide-canceled-expired-cowswap.vercel.app/orders/0xe16f5e497eed1a90d570542a365ad0791e42a23f1960acac4274312ba64e707a5be9a4959308a0d0c7bc0870e319314d8d957dbb6769acd3?tab=overview