Update Remove LP form -> New Design #1557
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
| ? (() => { | ||
| // Calculate the ratio of remaining shares to the total supply | ||
| const ratio = calculateRatio({ | ||
| a: lpShares - (lpSharesIn || 0n) - (withdrawalShares || 0n), // Remaining LP Shares in pool after removing input value lpShares and withdrawalShares received |
There was a problem hiding this comment.
Quick question: If a user has withdrawalShares, does that count as a part of the pool percentage? I'm including those in the ratio calculation here because i'm assuming they are removed from the pool along with the LP shares and treated differently.
There was a problem hiding this comment.
Yeah, your assumption is correct 👍
They are accounted for separately and don't contribute to the total pools liquidity.
| <label className="items-end text-neutral-content"> | ||
| {bottomRightStatistic ? bottomRightStatistic : null} | ||
| </label> | ||
| <div className="flex w-full items-baseline justify-end text-sm"> |
There was a problem hiding this comment.
This was bugging me:
Before items-baseline
After items-baseline:
| decimals: hyperdrive.decimals, | ||
| }); | ||
|
|
||
| const baseTokenDepositEnabled = |
| ? (() => { | ||
| // Calculate the ratio of remaining shares to the total supply | ||
| const ratio = calculateRatio({ | ||
| a: lpShares - (lpSharesIn || 0n) - (withdrawalShares || 0n), // Remaining LP Shares in pool after removing input value lpShares and withdrawalShares received |
There was a problem hiding this comment.
Yeah, your assumption is correct 👍
They are accounted for separately and don't contribute to the total pools liquidity.
| <div className="flex flex-col text-xs text-neutral-content"> | ||
| {`Withdrawable: ${formatBalance({ | ||
| balance: lpShares, | ||
| decimals: baseToken.decimals, | ||
| places: baseToken.places, | ||
| })}`} | ||
| </div> |
There was a problem hiding this comment.
nits:
- unnecessary class names
<div>is kinda like a catch all for block elements and<span>for inline. Even though we mostly use CSS to override these display properties, I still like using the tag that best fits the intention, i.e., the one I would use if writing HTML without CSS. Especially ✨semantic elements✨ when it makes sense.
| <div className="flex flex-col text-xs text-neutral-content"> | |
| {`Withdrawable: ${formatBalance({ | |
| balance: lpShares, | |
| decimals: baseToken.decimals, | |
| places: baseToken.places, | |
| })}`} | |
| </div> | |
| <span className="text-xs text-neutral-content"> | |
| {`Withdrawable: ${formatBalance({ | |
| balance: lpShares, | |
| decimals: baseToken.decimals, | |
| places: baseToken.places, | |
| })}`} | |
| </span> |
There was a problem hiding this comment.
Agreed, I try to use the approriate tag when needed. However, I've been reaching for divs so as to not cause any dom nesting issues in any of our components in the case I pass a <p><div>Text</div></p>. It should be fine here though.
| ? fixed(actualValueOut, hyperdrive.decimals).toString() | ||
| : "0" | ||
| } | ||
| maxValue={ | ||
| actualValueOut | ||
| ? formatUnits(actualValueOut, hyperdrive.decimals) |
There was a problem hiding this comment.
nit: 2 ways to do the same thing: formatUnits and fixed + toString. I recommend choosing 1. I'm indifferent to which.
This PR updates the design of the remove LP form.
Screen.Recording.2024-10-11.at.12.15.44.PM.mov