scaffold-eth / scaffold-eth-2

Open source forkable Ethereum dev stack
https://scaffoldeth.io
MIT License
1.41k stars 887 forks source link

Shadcn rainbowkit button + dropdown #937

Closed rin-st closed 2 months ago

rin-st commented 2 months ago

Description

Custom rainbowkit button + dropdown implemented with shadcn

See https://github.com/scaffold-eth/scaffold-eth-2/pull/937#discussion_r1760213449 and https://github.com/scaffold-eth/scaffold-eth-2/pull/937#discussion_r1760199979 first

Buttons functionality

https://github.com/user-attachments/assets/7d0ba4f7-7508-46c0-8b5f-ccc67f7000f2


With networks switch

https://github.com/user-attachments/assets/4d8693e2-df21-49b0-896e-3e270bea4c4e


Wrong network dropdown

https://github.com/user-attachments/assets/77ec7cb6-0074-40ee-ab7a-869b1a1aa74d


Dark theme

https://github.com/user-attachments/assets/2fd32a10-eb2f-4d4c-b8d5-8376eb83feeb

technophile-04 commented 2 months ago

Hey @rin-st thanks for such detailed explanation love it!

I feel like we shouldn't touch the components added from shadcn yet(unti will we are in dev phase) and let them be stock as it is. Umm I don't want us to mimic daisyUI SE-2 with shadcn and keep it as stock as possible also avoid passing of extra className while using them(until we are in development stage). Its completely fine if things breaks a bit / there very rough conner since things are not final yet.

^ We can always polish stuff later and as you mentioned in https://github.com/scaffold-eth/scaffold-eth-2/pull/937#discussion_r1760199979 we are also not final on theme yet. When we update we can always again visit this things. Also the reason I say we keep things as close to shadcn without updating much is this will make adding shadcn stuff to SE-2 look more compatible when people start using it.

For example if we update shadcn btn from rounded-md => rounded-full. Now if a developer using SE-2 plans to use Alert component from shadcn he has to manually update that component to bit more rounded.

what do you say? Thanks @rin-st!! Also please fee free to push changes directly to that branch 🙌

rin-st commented 2 months ago

For example if we update shadcn btn from rounded-md => rounded-full. Now if a developer using SE-2 plans to use Alert component from shadcn he has to manually update that component to bit more rounded.

It completely depends of our default styles. Like now, our buttons are always rounded, so when dev uses <button className='btn' /> its always rounded. So I just kept it as is now.

I feel like we shouldn't touch the components added from shadcn yet(unti will we are in dev phase) and let them be stock as it is. Umm I don't want us to mimic daisyUI SE-2 with shadcn and keep it as stock as possible also avoid passing of extra className while using them(until we are in development stage). Its completely fine if things breaks a bit / there very rough conner since things are not final yet.

But if we want to take default shadcn styles and change it later then ok, let's do it.

rin-st commented 2 months ago

Reverted ui changes and passing classNames

But I'm not sure regarding the last. We will need to do this anyway, so looks like it's better to keep that classNames from the start, so we will not forget to change it in future.

See, how it looks. With classnames:

https://github.com/user-attachments/assets/2021166f-513d-44d6-bf45-df013669dc88

Without classNames - trigger button padding, dropdown border, buttons width, disconnect color (we can make disconnect button type="destructive", but it will be with red bg, so not so good I think):

https://github.com/user-attachments/assets/92a531ae-1f38-4426-a717-bcbf58b22fd3

everything except buttons width is optional for now though

rin-st commented 2 months ago

Returned full width for menu items for now. Also fixed a bug, menu items were clickable during closing animation. Fixed version: see where I click to close the menu. Before fixing I'd be disconnected

https://github.com/user-attachments/assets/11f54dd5-8c43-4fc8-b27f-242369e849eb


Merging this, I think current state is good enough to move forward