-
Notifications
You must be signed in to change notification settings - Fork 45
fix: small pools filter showing at least 10 #1605
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
bc2eb66 to
7f80400
Compare
| // migration from v1 to v2: add deprecated filter | ||
| migrate: (oldValue, initial) => [...initial.filter((i) => !oldValue.some((o) => o.id === i.id)), ...oldValue], | ||
| } | ||
| const migration: MigrationOptions<ColumnFiltersState> = { version: 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.
no need for the migration anymore, we don't keep the defaultValues in local storage
…into fix/dex-small-pools
packages/curve-ui-kit/src/shared/ui/DataTable/hooks/useColumnFilters.tsx
Show resolved
Hide resolved
0xAlunara
left a comment
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 approved, but actually when going to https://curve-dapp-git-fix-dex-small-pools-curvefi.vercel.app/dex/plasma/pools?sort=-tvl I'm still seeing more than 10 pools, even though I've enabled "hide small pools" 🤔
It works for Ethereum, but not Plasma. Perhaps lite networks are treated differently?
EDIT: I'm trying other networks and it's just Plasma that doesn't seem to work, not sure what's going on here...
Well, the 10th pool there has 0 liquidity.. So all pools with the same liquidity are shown. I think that's kind of OK, because it would be pretty random to hide pools that have the same liquidity 🤔 and quite hard with the current solution. What do you think? |
Uh oh!
There was an error while loading. Please reload this page.