-
Notifications
You must be signed in to change notification settings - Fork 666
Fix: LP Fees shown currency amount #6605
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
Conversation
src/analytics/event.ts
Outdated
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.
FYI will need to reconstruct charts with assets instead of asset
const estimatedHorizontalPadding = 250; | ||
const maxChars = Math.floor((DEVICE_WIDTH - estimatedHorizontalPadding) / avgCharWidth); | ||
|
||
const NativeCurrencyDisplay = memo(function NativeCurrencyDisplay({ assets }: { assets: ClaimableType['assets'] }) { |
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.
this could use some more testing. Seemed good from my limited testing, but might be brittle with super long token names
export interface ClaimableAsset<T, A> { | ||
asset: T; | ||
amount: A; | ||
usd_value: number; | ||
value: string; | ||
} |
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.
different asset types and amount can be formatted or a raw string depending on where we are in the flow.
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.
Looks good, just one design nit and small type lint issue
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.
Top notch, no nits. Just curious about color reusability, non blocking.
backgroundColor={`rgba(41,90,247,1)`} | ||
disabledBackgroundColor={`rgba(41,90,247,0.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.
Is this worth popping into DS or is it definitely a 1-off?
const { nativeCurrency } = useAccountSettings(); | ||
const { isDarkMode } = useColorMode(); | ||
|
||
const color = !isDarkMode ? '#09111F' : globalColors.white100; |
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.
same question
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'll ask in standup, not sure tbh
* enable multi token LP claims * wrap up +N token logic and final tweaks * Update src/components/asset-list/RecyclerAssetList2/Claimable.tsx * bump claimables store v * fix up eth rewards types * remove unused nativeDisplay * remove useMemo for total value * finish merkl claimable title / subtile work * fix lint and code review suggestions
Fixes APP-2606
What changed (plus any additional context for devs)
Screen recordings / screenshots
What to test
all claimable types and entry points