-
Notifications
You must be signed in to change notification settings - Fork 429
feat: added dynamic assets categories using Coingecko API #2593
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
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
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.
waiting for automated categories but
hover seems off
Also i think would be more pretty to have search on left and picker right.
and full width on mobile (and check left and right padding is same as top bottom seems smaller not sure)
worth it adding in dashboard wherever we have more than X assets or assets in more than Y categories?
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
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.
AMAZING progress since the past iteration! would like to have others reviews as well as we have some design decisions to do before merging!
return await response.json(); | ||
}, | ||
queryKey: ['coingecko-categories'], | ||
staleTime: 1000 * 60 * 60 * 24, |
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.
maybe we can even cache the response for a week?
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.
done
pages/api/coingecko-categories.ts
Outdated
const [resEth1, resEth2, resEth3] = await Promise.all([ | ||
fetch(`${CG_ENDPOINT}?vs_currency=usd&category=liquid-staked-eth&per_page=250&page=1`, { | ||
method: 'GET', | ||
headers: HEADERS, | ||
}), | ||
fetch(`${CG_ENDPOINT}?vs_currency=usd&category=ether-fi-ecosystem&per_page=250&page=1`, { | ||
method: 'GET', | ||
headers: HEADERS, | ||
}), | ||
fetch(`${CG_ENDPOINT}?vs_currency=usd&category=liquid-staking-tokens&per_page=250&page=1`, { | ||
method: 'GET', | ||
headers: HEADERS, | ||
}), | ||
]); |
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.
we can make eth calls in parallel to stable to optimize
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.
done
const isOriginAllowed = (origin: string | undefined): boolean => { | ||
if (!origin) return false; | ||
|
||
if (allowedOrigins.includes(origin)) return true; | ||
|
||
// Match any subdomain ending with avaraxyz.vercel.app for deployment urls | ||
const allowedPatterns = [/^https:\/\/.*avaraxyz\.vercel\.app$/]; | ||
|
||
return allowedPatterns.some((pattern) => pattern.test(origin)); | ||
}; | ||
|
||
if (process.env.CORS_DOMAINS_ALLOWED === 'true') { | ||
res.setHeader('Access-Control-Allow-Origin', '*'); | ||
} else if (origin && isOriginAllowed(origin)) { | ||
res.setHeader('Access-Control-Allow-Origin', origin); | ||
} | ||
|
||
res.setHeader('Access-Control-Allow-Methods', 'GET, OPTIONS'); | ||
res.setHeader('Access-Control-Allow-Headers', 'Content-Type'); | ||
|
||
if (req.method === 'OPTIONS') { | ||
return res.status(200).end(); | ||
} |
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.
really nice!
.filter((res) => | ||
isAssetInCategoryDynamic( | ||
res.symbol, | ||
selectedCategory, | ||
stablecoinSymbols, | ||
ethCorrelatedSymbols | ||
) | ||
) |
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! so clean
ETH_CORRELATED = 'eth_correlated', | ||
} | ||
|
||
export const STABLECOINS_SYMBOLS_FALLBACK = [ |
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.
interesting. so i imagine the effect if the request takes time and I've selected a category will see the list to update .e.g. adding more items because coingecko has more than the fallback, wondering if it makes sense to have a fallback or just disable the picker until we have data. Plus, we can cache the data in the localStorage for a long time, since categories wont change frequently.
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.
Understood. I was thinking it would be a good idea to always render something for the user, for example, if the CoinGecko API is down. That said, I agree the other approach makes sense as well. Let’s make a final decision with the rest of the team, and I can update it, no problem.
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
General Changes
Added dynamic categories and asset filters on the Market page using the Coingecko API. We can now filter by: All, Stablecoins, or ETH-Correlated.
Improved mobile view with layout modifications.
Added dynamic categories and asset filters also in Dashboard page.
Developers Notes:
COINGECKO_API_KEY=
Reviewer Checklist
Please ensure you, as the reviewer(s), have gone through this checklist to ensure that the code changes are ready to ship safely and to help mitigate any downstream issues that may occur.
.env.example
file as well as the pertinant.github/actions/*
files