-
Notifications
You must be signed in to change notification settings - Fork 17
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
Redesign Duration display for Dutch auctions with live-updating price visualization. #1092
Conversation
5b5e865
to
6b6c273
Compare
6b6c273
to
f481c41
Compare
f481c41
to
3696baa
Compare
size='sm' | ||
className='mt-2' | ||
/> | ||
<div className='mt-2'> |
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.
I removed className
from <Progress />
's props, so we need to wrap it in a div with a margin-top.
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.
Nice! Looks good.
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! Another case for #965.
const p = targetOutputScaled; | ||
const q = inputScaled; |
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.
Do we need these reassigned? Can we just use targetOutputScaled
and inputScaled
inline below?
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, good catch — this was a leftover from copying core's implementation.
@@ -0,0 +1,16 @@ | |||
const clampToDecimal = (value: number) => Math.min(Math.max(value, 0), 1); |
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.
What does clamp mean here? Maybe a code comment here? (same in other usage too)
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.
Clamping refers to limiting a value between a given min and max. Will add a comment.
Scrub through this video to get a timelapse of part of a 10-minute auction.
Screen.Recording.2024-05-10.at.5.41.55.PM.mov
This is a bit of a frilly PR that I prob shouldn't have spent as much time on as I did... Basically it visualizes to the user what the current price is of the input asset in terms of the output asset, as well as how far along we are in the auction.
Again, this is by no means a finalized design, but it's at least an improvement on what we had before.
In this PR
status
slice for storing the sync status (fullSyncHeight
andlatestKnownBlockHeight
) so it can be used both in the header and in places like Dutch auctions.<Progress />
to only accept a few specific props, rather than the full default set (which we neither need nor want).<AssetIcon />
and<ValueViewComponent />
.Closes #1088