Chore(dev hub) move pf pro #3091
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
apps/developer-hub/src/components/Pages/PriceFeedsLandingPage/index.tsx
Outdated
Show resolved
Hide resolved
| const filteredFeeds = useMemo(() => { | ||
| return priceFeeds.filter((feed) => { | ||
| const searchLower = search.toLowerCase(); | ||
| return ( | ||
| feed.symbol.toLowerCase().includes(searchLower) || | ||
| feed.name.toLowerCase().includes(searchLower) || | ||
| feed.description.toLowerCase().includes(searchLower) || | ||
| feed.pyth_lazer_id.toString().includes(searchLower) | ||
| ); | ||
| }); | ||
| }, [priceFeeds, search]); |
There was a problem hiding this comment.
We have this searching lib that's quite nice https://github.com/pyth-network/pyth-crosschain/blob/main/apps/insights/src/components/Root/search-button.tsx#L146-L153 would be great to use this
| onChange={updateSearch} | ||
| className="w-full p-2 mb-4 border border-gray-300 rounded-md" | ||
| /> | ||
| <Table<Col> |
There was a problem hiding this comment.
the page seems to be lagging when all the feeds are displayed, I think it would be nice to have some sort of pagination. I reccomend using a similar approach to here:
this manages filtering, sorting and pagination https://github.com/pyth-network/pyth-crosschain/blob/main/apps/insights/src/components/PriceFeeds/price-feeds-card.tsx#L84 (https://insights.pyth.network/price-feeds)
this is the bottom pagination component: https://github.com/pyth-network/pyth-crosschain/blob/main/apps/insights/src/components/PriceFeeds/price-feeds-card.tsx#L285-L292
| useEffect(() => { | ||
| const timer = setTimeout(() => { | ||
| setSearch(searchRaw.trim()); | ||
| }, 300); | ||
| return () => { | ||
| clearTimeout(timer); | ||
| }; | ||
| }, [searchRaw]); |
There was a problem hiding this comment.
this is good, but I think the main heavy thing we do here is about UI rendering the whole list, sorting through 2000-3000 js objects is not that heavy.
If we have pagination this is not needed anymore
There was a problem hiding this comment.
This is also a very strange way to do debouncing / throttling and it's a bit of an abuse of effects, try using react-hookz instead, it's quite nice
| }: ProductCardProps) { | ||
| return ( | ||
| <div | ||
| className={`bg-white dark:bg-gray-800 rounded-lg border border-gray-200 dark:border-gray-700 p-6 shadow-sm hover:shadow-md transition-shadow flex flex-col h-full ${className}`} |
There was a problem hiding this comment.
this is also tailwind, also can we please use clsx for appending classNames
|
|
||
| --- | ||
|
|
||
| <div className="grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-3"> |
There was a problem hiding this comment.
I think we should avoid tailwind and use sass for anything we write -- tailwind is used internally by fumadocs but all of this would break if we were to e.g. move to a different markdown renderer.
We should probably see if we can configure the fumadocs built-in tailwind to not look at the content folder...
| @media (width >= 640px) { | ||
| .container { | ||
| padding-left: theme.spacing(6); /* sm:px-6 */ | ||
| padding-right: theme.spacing(6); | ||
| } | ||
| } | ||
|
|
||
| @media (width >= 1024px) { | ||
| .container { | ||
| padding-left: theme.spacing(8); /* lg:px-8 */ | ||
| padding-right: theme.spacing(8); | ||
| } | ||
| } |
There was a problem hiding this comment.
You should be nesting these inside .container, and you should use theme.breakpoint for these, e.g. this:
.container {
max-width: ...;
@include theme.breakpoint("sm") {
...
}
}| @use "@pythnetwork/component-library/theme"; | ||
|
|
||
| .container { | ||
| max-width: 80rem; /* ~ max-w-7xl */ |
There was a problem hiding this comment.
Please stick to theme.spacing for size units
| max-width: 80rem; /* ~ max-w-7xl */ | ||
| margin-left: auto; | ||
| margin-right: auto; | ||
| padding: theme.spacing(8) theme.spacing(4) theme.spacing(8) theme.spacing(4); /* px-4 */ /* py-8 */ |
There was a problem hiding this comment.
This is more concise as padding: theme.spacing(8) theme.spacing(4);
| } | ||
|
|
||
| .title { | ||
| font-size: 2.25rem; /* text-4xl */ |
There was a problem hiding this comment.
Please use @include theme.text() for font sizes & weights etc
| } | ||
|
|
||
| .link:hover { | ||
| color: var(--color-blue-800); |
There was a problem hiding this comment.
Don't color vars -- these shouldn't even exist and I think they're defined internally in fumadocs or something -- stick to colors from theme.color
| <ProductCard | ||
| badge="Pro" | ||
| badgeColor="bg-blue-600" | ||
| icon={<LightningIcon size={24} />} |
There was a problem hiding this comment.
Minor nit, but generally I don't like passing sizing to icons from html, I instead prefer setting font-size in css -- this is after all part of styling, not content.
| <div className={styles.gridTwo}> | ||
| <ProductCard | ||
| badge="Pro" | ||
| badgeColor="bg-blue-600" |
There was a problem hiding this comment.
We should not be passing around tailwind class names like this -- as I'd mentioned above, tailwind is internal to fumadocs and we shouldn't use it in stuff we write.
|
|
||
| [[package]] | ||
| name = "pyth-lazer-protocol" | ||
| version = "0.14.0" |
There was a problem hiding this comment.
There should not be changes to this file in this PR, is this intentional?
| useEffect(() => { | ||
| const timer = setTimeout(() => { | ||
| setSearch(searchRaw.trim()); | ||
| }, 300); | ||
| return () => { | ||
| clearTimeout(timer); | ||
| }; | ||
| }, [searchRaw]); |
There was a problem hiding this comment.
This is also a very strange way to do debouncing / throttling and it's a bit of an abuse of effects, try using react-hookz instead, it's quite nice
Summary
Rationale
How has this been tested?