Portfolio Onboarding Section #391
Conversation
| const [isOpen, setIsOpen] = useState(false); | ||
| const { data: providerManifests } = useProviderManifests(); | ||
| const providerOrigins = useMemo(() => Object.keys(PenumbraClient.getProviders()), []); | ||
| const { isPenumbraConnected, isCosmosConnected } = useUnifiedAssets(); |
There was a problem hiding this comment.
suggestion: can we refactor the cosmos connection status logic in useUnifiedAssets into a separate hook (eg. useConnectionStatus.tsx) to reduce scope? useUnifiedAssets definitionally is more focused on asset management, which includes other expensive computations like balance aggregation and price fetching.
#380 can then modify useUnifiedAssets to use that hook accordingly. That way, we can remove all direct references to use-unified-assets.tsx and use-augmented-balances.tsx here. cc @vacekj
| const providerOrigins = useMemo(() => Object.keys(PenumbraClient.getProviders()), []); | ||
| const { isPenumbraConnected, isCosmosConnected } = useUnifiedAssets(); | ||
|
|
||
| const onConnectClick = () => { |
There was a problem hiding this comment.
comment: experiencing CORS, 401 unauthorized, and 404 not found errors in the dev logs when connecting to the keplr wallet – could this be due to it attempting to connect on localhost?
Screen.Recording.2025-03-14.at.2.31.54.PM.mov
| * Uses the same summaries API that the explore page uses | ||
| * Only returns prices denominated in USDC | ||
| */ | ||
| export const useAssetPrices = (assets: Metadata[] = []) => { |
There was a problem hiding this comment.
ACK – this diverges from and provides a more flushed out impl than https://github.com/penumbra-zone/dex-explorer/pull/380/files#diff-2a848464ce5b56c4d4661b0e5ca75fa722410c0793592b77fd566d911b2c50d0R10. cc @vacekj for visibility
|
|
||
| const onConnectClick = () => { | ||
| if (providerOrigins.length > 1) { | ||
| setIsOpen(true); |
There was a problem hiding this comment.
comment: we should avoid potentially triggering a state update during rendering – I think this violates React rules and should use useEffect instead?
| if (priceData?.quoteSymbol === 'USDC' && !isNaN(priceData.price)) { | ||
| // Use price data if available | ||
| assetValue = numericAmount * priceData.price; | ||
| } else if (isStablecoinSymbol(metadata.symbol)) { |
There was a problem hiding this comment.
suggestion: check if metadata is undefined
if (metadata && isStablecoinSymbol(metadata.symbol)) { ... }| // @ts-expect-error type error, but it works. | ||
| import '@interchain-ui/react/styles'; |
| durationWindow: '1d', // Use 1-day window | ||
| }); | ||
| }, | ||
| staleTime: 60000, // 1 minute |
There was a problem hiding this comment.
comment: should we fetch prices more frequently? I think it may be fine for now
Implements #387