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

fix: limit order list sorted by creation date #8484

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

woodenfurniture
Copy link
Member

Description

Fixes issue where orders in the list appear to be in random order.

Issue (if applicable)

https://discord.com/channels/554694662431178782/1324596242394644571/1325626856069660756

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

Create some limit orders, note what order you did them in.
Wait for them to execute, or cancel them.
View the historical orders

Engineering

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

See https://discord.com/channels/554694662431178782/1324596242394644571/1325626856069660756

Screenshots (if applicable)

Created a fresh order for AIDOGE. Should appear at top of liste.

develop:

this pr:

@woodenfurniture woodenfurniture requested a review from a team as a code owner January 7, 2025 02:59
@gomesalexandre gomesalexandre self-requested a review January 7, 2025 08:35
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM! Note the diff orders in list because CoW doesn't aggregate across EVM chains.

  • CoW UI
image image
  • This diff
image image

Two remarks of weird UX, unrelated to this diff per se @woodenfurniture

  1. We sould probably have a place where we can display created date. Feels kind of odd not being able to see it anywhere at all.

CoW has this "Order Receipt" view which contains all meta/data about the order, not sure if we would like to add something like this after we implement #8429?

image
  1. Asset icons should display the network icon, since CoW exists on multiple chains, and there is no way to visually discriminate orders as-is

@gomesalexandre gomesalexandre enabled auto-merge (squash) January 7, 2025 16:45
@gomesalexandre gomesalexandre merged commit 9bedb7b into develop Jan 7, 2025
3 checks passed
@gomesalexandre gomesalexandre deleted the fix-order-list-sorting branch January 7, 2025 16:52
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.

2 participants