-
Notifications
You must be signed in to change notification settings - Fork 1
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
[APR] PoolTable Filter #251
Conversation
BAL-710 UI for filter on poolTable
input filter poolTable
|
would take a look into https://ui.shadcn.com/docs/components/combobox |
@ribeirojose I got the Downshift recommendantion from this thread on shadcn's repo about multi-select |
2ac1767
to
df86cb5
Compare
✅ Deploy Preview for balancer-tools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Cool! Haven't tried out the PR yet but just pointed it as a resource. This comment from the thread you mentioned is 🔥 shadcn-ui/ui#66 (comment) |
apps/balancer-tools/src/app/apr/round/(components)/TokenFilterInput.tsx
Outdated
Show resolved
Hide resolved
apps/balancer-tools/src/app/apr/round/(components)/TokenFilterInput.tsx
Outdated
Show resolved
Hide resolved
This reverts commit 435e80a.
Co-Authored-By: José Ribeiro <32641844+ribeirojose@users.noreply.github.com>
apps/balancer-tools/src/app/apr/round/(components)/PoolTableWrapper.tsx
Outdated
Show resolved
Hide resolved
eaac9ad
to
50e60a0
Compare
apps/balancer-tools/src/app/apr/(components)/MultiSelectDropdown.tsx
Outdated
Show resolved
Hide resolved
apps/balancer-tools/src/app/apr/(components)/MultiSelectDropdown.tsx
Outdated
Show resolved
Hide resolved
95d5708
to
f88a9a1
Compare
return ( | ||
<Badge key={value + idx} color="blue"> | ||
<div | ||
className="flex gap-2 items-center w-max" |
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.
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.
gap-1 would reduce the gap tho
apps/balancer-tools/src/app/apr/(components)/MultiSelectDropdown.tsx
Outdated
Show resolved
Hide resolved
); | ||
|
||
return ( | ||
<div className="relative"> |
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.
Shouldn't it wrap at some point and have some max width/height? How would this show on mobile? e.g. https://tools-monorepo-dxnj3a8mv-ribeirojose.vercel.app/apr/round/74?tokens=WETH%2CsdBal%2CB-80BAL-20WETH%2CjEUR%2Cbb-USD%2B%2CwstETH%2CDOLA%2CwBETH%2CwjAURA%2CswETH%2CstaBAL3%2CauraBAL%2CagEUR%2CPAR
![Screenshot 2023-08-31 at 11 57 08](https://private-user-images.githubusercontent.com/32641844/264687013-a4d3d9b5-7115-4fc8-826d-a04901c31669.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjE4MzEyMzQsIm5iZiI6MTcyMTgzMDkzNCwicGF0aCI6Ii8zMjY0MTg0NC8yNjQ2ODcwMTMtYTRkM2Q5YjUtNzExNS00ZmM4LTgyNmQtYTA0OTAxYzMxNjY5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA3MjQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNzI0VDE0MjIxNFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTY4ODVkNjBjZTBkNWVlM2NmYTllYjljY2FmYjViNzMwN2QzYTdkNjE0OGZlZDkxNjRjZDUzYzllOTg3NDZmMWYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.kUIG5CH8N1EZslg4R3vmplVyIhDX2nDNH_k6oJVbL9U)
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.
From that above example ☝️ we also need an empty state when that search doesn't yield any results. Can you track that on Linear?
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 was thinking about it, do you think we should limit number os tokens to be searched at once?
cc @luizakp
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 just checked Balancer UI and they don't handle this case as well... from all the pool type the max amount of tokens is 8. So I think it makes sense to limit 8 tokens.WDYT?
|
||
return ( | ||
<div className="relative"> | ||
<Downshift {...rest} onChange={changeHandler()}> |
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.
Keyboard arrow navigation doesn't work for this Select 😞 no need to work on this now since we've already put a lot of time here, but it's certainly something that's very intuitive to me that I couldn't use here
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.
👍 Downshift should help with that, I'll take a stab at it later
apps/balancer-tools/src/app/apr/(components)/MultiSelectDropdown.tsx
Outdated
Show resolved
Hide resolved
@ribeirojose @luizakp Need you guys' approval |
Altered calculatePoolStats to return tokens used on Pool
Add PoolType return to calcutePoolStats/Route
Altered PoolListTable column headings
Added Downshift as a dependency
Implemented Multi selector with Downshift
Implemented TokenFilter with Multi selector component
Implemented util function to generate API url with filters
Changed token filter name and added Type
Better Ux on MultiSelect
Refactor PoolTable to use queryParams to sort and order
Adds initial min tvl and limit on filter url
Implemented function to filter selected items
Moved MultiSelectDropdown to components folder
Smaller text on the popover
Add max width to MultiSelect
Add empty state to poolTable
Added hasMorePool conditional to PoolTable