-
Notifications
You must be signed in to change notification settings - Fork 182
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: add limit order view UI #8020
Conversation
src/components/MultiHopTrade/components/SharedTradeInput/SharedTradeInput.tsx
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.
Overall looking super tasty, with just a couple minor comments to re-use things and translate stuff where we can 🎉
Q: Is the layout for narrow screens implemented yet, or is out of scope? Currently on narrow screens it doesnt show the order view button and is far left:
Non-blocking (since this is all WIP): Need to be able to scroll the orders list
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Outdated
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Outdated
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Outdated
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Outdated
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Outdated
Show resolved
Hide resolved
24a5e9f
to
a8cc142
Compare
Indeed, that was not intended for this PR 🙏 "This PR does not yet handle smaller viewports, only the expanded version".
Good catch, I'll address this in the next PR. |
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.
a few non-blocking comments and 1 seemingly missed blocking one
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Outdated
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/components/LimitOrderCard.tsx
Show resolved
Hide resolved
src/components/MultiHopTrade/components/LimitOrder/LimitOrderList.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.
getitin
Description
Adds UI for viewing open and completed limit orders.
Note for reviewer:
AssetIconWithBadge
component to properly handle the use case (follow-up PR), and so it's currently hardcoded to the Ethereum icon for mock purposesIssue (if applicable)
Contributes to #6206
Risk
Small, all changes are behind a flag and visible only on the Limit tab.
None
Testing
Ensure when clicking the limit tab the
OrdersList
component opens automatically, and matches the mocks.Engineering
☝️
Operations
Screenshots (if applicable)