feat(react-ui): SelectDropdown component#321
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
OmarBasem
changed the title
OmarBasem
changed the title
OmarBasem
changed the title
179f991 to
cf04c8c
Compare
809972b to
72405d7
Compare
72405d7 to
dd32b76
Compare
cf04c8c to
d10bcf1
Compare
d10bcf1 to
e7f7f94
Compare
030f2e7 to
c55bfa9
Compare
e7f7f94 to
252b392
Compare
252b392 to
ec6950c
Compare
ec6950c to
1c0851b
Compare
997be70 to
1acb066
Compare
| @@ -0,0 +1,216 @@ | |||
| import { | |||
There was a problem hiding this comment.
The Default story looks a bit off. It might be just an issue with the story file layout itself
1c0851b to
45a13b3
Compare
1acb066 to
8473f8f
Compare
9d5d550 to
ff3ef31
Compare
9ecdc9b to
68fd347
Compare
ff3ef31 to
6858226
Compare
68fd347 to
bce9516
Compare
6858226 to
48c50b6
Compare
bce9516 to
c06d635
Compare
48c50b6 to
8f7972b
Compare
8f7972b to
ff9b411
Compare
ff9b411 to
cfc4ad5
Compare
commit: |
# Conflicts: # packages/react-ui/src/components/DataRow/index.tsx # packages/smart-routing-address-react-ui/src/components/LabeledValueRow/LabeledValueRow.stories.tsx # Conflicts: # packages/smart-routing-address-react-ui/src/pages/Deposit.tsx # Conflicts: # packages/react-ui/src/components/PillItem/PillItem.test.tsx # packages/react-ui/src/components/PillItem/index.tsx
73bd63d to
4758aea
Compare
|
@andrascodes @brtkx thanks for your reviews, I have refactored to using Radix |
| ) | ||
|
|
||
| return ( | ||
| <Popover.Root> |
There was a problem hiding this comment.
This is not the right component - we should be using https://www.radix-ui.com/primitives/docs/components/select
There was a problem hiding this comment.
I'd just call it Select - it's pretty standard for UI libraries.
| } | ||
|
|
||
| const trigger = ( | ||
| <PillItem |
There was a problem hiding this comment.
I feel like PillItem is quite out of place in react-ui in general. It looks like a disabled select trigger - I would just move it back to SRA. Even if it's used in other packages later, it's just icon and text aligned horizontally. IMO this doesn't justify being in the shared package.
There was a problem hiding this comment.
I don't think we should be using radix this way. We should do it shadcn style where we export styled radix components and the consumer does what it wants with them. With the current implementation the code is bloated (a lot of props), and the consumer can only build token related components with it.
https://github.com/zerodevapp/zerodev-dashboard/blob/main/src/components/ui/select.tsx
ZeroDev dashboard is a good example how Select component has been exported.
Also can see here how shadcn exports radix components:
https://github.com/shadcn-ui/ui/blob/main/apps/v4/registry/bases/radix/ui/select.tsx
Feel free to take some time to dive deep into other projects, including ZeroDev dashboard, and check out how they export and consume radix.
There was a problem hiding this comment.
#322
For example the other component you've implemented is quite nice, one thing we can do is get rid of props by utilizing radix to its full potential. Generally radix allows us to avoid using monolith components - so that the consumer can build anything with styled blocks instead of passing props.
3 participants
Closes FS-2398