Make components more customizable
Closed this issue · 16 comments
Description :
Currently, we have the following user-facing components at SE-2 https://docs.scaffoldeth.io/components.
The idea is to make them more customizable but also they follow some standardization so it's intuitive while using any SE-2 component.
My thoughts:
I think the best part of SE-2 is we have components which are not abstracted in any library and they are part to repo directly.
So if people need to do some hardcore customization specific to their need they can always go to Address.tsx file and add the necessary styles directly or accepts more props if needed.
But yup I agree that having some basic props to more customize the components would be great for beginners and easy to use.
The approach we followed at #773 kind of adding {element}ClassName not sure if it's the best at current moment since it might get non intuitive for users to know which {elementClassName} goes to which element and they might need to look at the code of that Component to know which className goes to which element.
I think wrapperClassName kind of make sense but getting more granular than that might get non intuitive.
Props like size seems more intuitive to me and maybe we can think of some more on this line. Like maybe borderRadius to input components etc.
Maybe in case in future if we plan to abstract components in library then we might need to go with approach of {elementClassName} but I was looking for alternatives and love how shadcn/ui is doing it.
So instead of having only Alert component and then passing {element}ClassName to it as props, they ask you to do something like this :
<Alert>
<AlertTitle>Heads up!</AlertTitle>
<AlertDescription>
You can add components to your app using the cli.
</AlertDescription>
</Alert>This way you can pass className directly to AlertTitle & AlertDescription components and you know which className goes to which element.
Lol but yeah not sure if its best suited for our components but might make sense once we have lot of big components
This is a good comparison to look at:
The iPhone is elegant and intuitive which makes it very easy to use and understand. However, there is almost no room for customization; thus everything is forced to look and feel really cookie cutter.
Meanwhile, Android phones provide the user with many customizable options giving them flexibility and freedom, albeit requires more work on the user to do the setup and have a better understanding of the underlying technology.
Both phones are great, but serve their different purposes based on the user's needs. For instance, I have an iPhone because I like my phone to be simple and intuitive, meanwhile I prefer windows for my PC as I do like the granular control when I'm programming or getting into the finer details of the OS. Pretty much everyone is going to have different preferences.
So my question is: should SE2 be the iPhone or the Android for developing Dapps?
I personally think that it could be a mix of both. It can provide elegance and intuitiveness, while at the same time it is flexible enough to allow full customizations.
Solutions
ScaffoldAddress/BaseAddress
I’d like to highlight the PR’s proposed solution which was having two components ‘BaseAddress’ and ‘ScaffoldAddress’. Where ‘BaseAddress’ allows for full customization and ‘ScaffoldAddress’ maintains the elegance and intuitiveness.
Seperated Components
So instead of having only Alert component and then passing {element}ClassName to it as props, they ask you to do something like this :
<Alert> <AlertTitle>Heads up!</AlertTitle> <AlertDescription> You can add components to your app using the cli. </AlertDescription> </Alert>This way you can pass className directly to AlertTitle & AlertDescription components and you know which className goes to which element.
This seems to be another decent solution! I'm not sure which solution is better. Hopefully others can chime in with their thoughts!
Having a customizable BaseAddress component and then another simpler component sounds great to me!
I think we should call ScaffoldAddress just Address, so we don't have to change anything in the code (except changing this component implementation using the new BaseAddress component).
It seems like a good option for other components, having the Base component with more customization, so an advanced user that wants to change it can use the Base component (and if not, you can just use the current component).
Hey all, thanks for your take on this!
My take:
A lot of it makes sense to me If we were talking about extracting to external packages (@se-2/address, or @se-2/components). In SE-1 they were extracted (hooks too) and I have to say that it was a pain in the ass when I wanted to make a tweak to it (patches, making PRs to the lib + updating the version number). I guess that's the reason (+ extra maintainability) that we haven't take that path (at least for now). Everything needs to be super customizable (and in the right way)
If we are talking about improving the current state of the components (inside SE-2) a lot of this sounds like overcomplicating things + bloating the codebase...
So if people need to do some hardcore customization specific to their need they can always go to Address.tsx file and add the necessary styles directly or accept more props if needed.
100 % this. I've created dozens of projects with SE-2 (that don't look like SE-2!), and there is no better customization than editing the de component itself if you need it. I think this is also the reason why things like shadcn are getting popular. Super hard to get a UI component to look exactly how you want using a 3rd party lib.
In any case, I think it makes sense to rethink some of the props & display for the components. Maybe having a more standardized way of using the components (sizes, classNames, wrapperClassNames?). Anything that makes the codebase simpler / cleaner + more intuitive to use.
Hey all, thanks for your take on this!
My take:
A lot of it makes sense to me If we were talking about extracting to external packages (@se-2/address, or @se-2/components). In SE-1 they were extracted (hooks too) and I have to say that it was a pain in the ass when I wanted to make a tweak to it (patches, making PRs to the lib + updating the version number). I guess that's the reason (+ extra maintainability) that we haven't take that path (at least for now). Everything needs to be super customizable (and in the right way)
If we are talking about improving the current state of the components (inside SE-2) a lot of this sounds like overcomplicating things + bloating the codebase...
So if people need to do some hardcore customization specific to their need they can always go to Address.tsx file and add the necessary styles directly or accept more props if needed.
100 % this. I've created dozens of projects with SE-2 (that don't look like SE-2!), and there is no better customization than editing the de component itself if you need it. I think this is also the reason why things like
shadcnare getting popular. Super hard to get a UI component to look exactly how you want using a 3rd party lib.
Yes, but sometimes you need the same component with different styles or behaviors, so having some customization available in the component is useful.
In any case, I think it makes sense to rethink some of the props & display for the components. Maybe having a more standardized way of using the components (sizes, classNames, wrapperClassNames?). Anything that makes the codebase simpler / cleaner + more intuitive to use.
Yes, I think this is the main issue to resolve because if you look at the current components some of them have a className property, another one has a size property, and another one does not have any style-related property.
Anyway, as you said, having the components code there, the users can make any change they want, so I think that making these changes is not so important right now.
Yes, but sometimes you need the same component with different styles or behaviors, so having some customization available in the component is useful.
This has happened to me too. Usually I just add an extra prop... Or even I duplicate the component if the behavior is too different.
Getting the correct abstraction & level of customizability is super hard and you almost never get 100% what you want. That's what I think simple components that you can tinker with (again, thinking that components are inside SE2 codebase, not in a external lib) are the way to go for SE-2.
Let's make them better tho!
Really glad to see all the discussion surrounding this topic!
@carletex Those are really fair points and it makes sense not to go entirely down that route.
Regardless, I think as we continue to see Dapps evolve and get more complex, then the SE2 codebase should be able to support the complexity. I've already ran into a few situations where I've had to deviate from the SE2 hooks and interact directly with Wagmi/Viem. Which is fine, but damn it would've been nice to continue to use SE2 hooks for their effectiveness/usability. The frontend components should follow the same principles too IMO.
Anywho, sounds like we need to find a less intrusive and complex way of making more customizable components. I'll investigate a bit and see if I can cook something good up!
Regardless, I think as we continue to see Dapps evolve and get more complex, then the SE2 codebase should be able to support the complexity.
I think SE-2 supports creating complex projects pretty well, since the codebase is clean & simple (so it's easy to understand & tweak & extend). If you over-engineer components / hooks / utils, you end with a more complex codebase (harder to understand & harder to maintain) and a high chance that you end with some bad abstractions (that won't cover all the use cases either). Also, not beginner-friendly, which is one of the key points for SE-2.
I think that it's why we closed #773, it started concise (just tweaking the some prop in the Address component) but then it derived in 11 files changed PR with component renaming, new components and some code smells. If we did that for every component / hook the SE-2 codebase will be rough.
(Please don't take this personally, just trying to give you some more context about SE-2 that might help for future contributions.)
I've already ran into a few situations where I've had to deviate from the SE2 hooks and interact directly with Wagmi/Viem. Which is fine, but damn it would've been nice to continue to use SE2 hooks for their effectiveness/usability. The frontend components should follow the same principles too IMO.
I know that this conversation is about components, but I'd love to hear more about specific details in these situations! (we have made a bunch of tweaks/additions while tinkering with SE-2 and realising what we could improve). What did you try to accomplish? What SE-2 hook functionality was missing and you wanted to be there?
Thanks!
Anywho, sounds like we need to find a less intrusive and complex way of making more customizable components. I'll investigate a bit and see if I can cook something good up!
Copying from a previous comment, this could be a first good iteration:
In any case, I think it makes sense to rethink some of the props & display for the components. Maybe having a more standardized way of using the components (sizes, classNames, wrapperClassNames?). Anything that makes the codebase simpler / cleaner + more intuitive to use.
In any case, I think it makes sense to rethink some of the props & display for the components. Maybe having a more standardized way of using the components (sizes, classNames, wrapperClassNames?). Anything that makes the codebase simpler / cleaner + more intuitive to use.
Had the similar thoughts reviewing last pr's.
The approach we followed at #733 kind of adding {element}ClassName not sure if it's the best at current moment since it might get non intuitive for users to know which {elementClassName} goes to which element and they might need to look at the code of that Component to know which className goes to which element.
I had experience with that kind of props, and every time it became unmaintainable chaos, so I'd want to avoid it.
Maybe in case in future if we plan to abstract components in library then we might need to go with approach of {elementClassName} but I was looking for alternatives and love how shadcn/ui is doing it.
So instead of having only Alert component and then passing {element}ClassName to it as props, they ask you to do something like this :
<Alert>
<AlertTitle>Heads up!</AlertTitle>
<AlertDescription>
You can add components to your app using the cli.
</AlertDescription>
</Alert>I also have experience working with shadcn and I agree that this approach is much better. Shadcn is basically abstraction over radix UI + tailwind (alert example) . It's easy to customize but sometimes it requires good React knowledge.
I'll review the components next days and try to think about possible solutions. Yes, I think we need to standardize props. Or use shadcn so users can customize whatever they want
So if people need to do some hardcore customization specific to their need they can always go to Address.tsx file and add the necessary styles directly or accept more props if needed.
100 % this. I've created dozens of projects with SE-2 (that don't look like SE-2!), and there is no better customization than editing the de component itself if you need it. I think this is also the reason why things like
shadcnare getting popular. Super hard to get a UI component to look exactly how you want using a 3rd party lib.
I think we need to add to docs recipes for changing current and creating new components. I believe when we start to write it we will find things we can improve
Other things to consider about components (not so related to customizing, we can create new discussions for it):
- we need to try to create some simple rules if we need to add a new component or not. Sometimes it's not clear example (yes, it needs some changes like bigint amount). I think we need to add even simple components, it's much easier than creating recipes for that situation and it's better than nothing.
- components that users often need in web3 apps but we don't have it
(Please don't take this personally, just trying to give you some more context about SE-2 that might help for future contributions.)
Not taken personally at all! I came running head first, when I should have gathered context beforehand. So I do apologize for that.
I know that this conversation is about components, but I'd love to hear more about specific details in these situations! (we have made a bunch of tweaks/additions while tinkering with SE-2 and realising what we could improve). What did you try to accomplish? What SE-2 hook functionality was missing and you wanted to be there?
Apologies that this example is a bit messy. I don't have the time to clean up the code to make it more beautiful to look at. I will gladly run anyone through it in a deeper manner if you would like! Anyway, on this project, there were several things that SE2 couldn't properly support.
Part 1
Originally, in my deploy script (you can see it commented out), I had deployed 28 instances of the SONG.sol smart contract. SE2 would only transfer over the last deployed SONG.sol, leaving the other 28 in the dust. They weren't accessible through SE2 hooks and there was no way to reference them.
My initial solution was to do something like SONG1.sol, SONG2.sol, SONG3.sol, etc... However, it especially became tedious when I started worrying about 28 different songs. Imagine trying to support hundreds, thousands, or millions. The number of unique smart contracts became abundant and the number of hooks became abundant (useScaffoldContractRead(song1), useScaffoldContractRead(song2), useScaffoldContractRead(song3), etc...). And there was no way for these hooks to work in a truly dynamic way. I was longing for something like useScaffoldContractRead(getAllSongs()) - rough psuedocode lol.
Luckily, I did have a PLAYLIST.sol smart contract which kept track of them inside a linked list. However, this only kept track of the smart contracts' addresses, thus using SE2 I had to reference the PLAYLIST.sol instance, grab the addresses, and convert them into smart contract objects using Wagmi/Viem.
Part 2
For some unrelated reason (I think gas costs), I had to switch my whole operation to create and include SONGFACTORY.sol, a smart contract which allows you to deploy new instances of SONG.sol. Thus, driving me even further from SE2. I now removed deploying the instances of SONG.sol from Deploy.s.sol and had put the onus on the frontend to interact with the factory to create new instances and add them to PLAYLIST.sol (Small aside, I believe I had to go this route as deploying the 28 smart contracts in the DeployScript was causing issues with some of the transactions getting dropped from the mempool). Then of course, since I had to interact with the frontend to create new instances of SONG.sol through SONGFACTORY.sol, and additionally adding those instances to PLAYLIST.sol. Then, it kept me on the route of grabbing the addresses from PLAYLIST.sol and converting them to objects using Wagmi/Viem.
Some loose thoughts
This might not be the best explanation but: SE2 works phenomenally with 1 to 1 relationships, but suffers a bit in 1 to many relationships.
It would be nice to do something like useScaffoldContractRead({ contractName: "Song", contractAddress: "0x123", functionName: "tokenURI"}). Where you can override the address, but still reference the same abi.
Maybe I'm missing the context of SE2 here. Is it meant to be a full solution from start to finish (building/deploying smart contracts to a fully functional frontend). Or does it act more as a debugging/early prototyping tool where you are expected to take the training wheels off at some point and roll your own frontend using Wagmi/Viem?
I'll review the components next days and try to think about possible solutions. Yes, I think we need to standardize props. Or use shadcn so users can customize whatever they want
Sure, don't go too crazy about it tho haha. I think we can start with some basic iterations of cleaning/standardizing.
Agree with all that you are saying except:
I think we need to add even simple components, it's much easier than creating recipes for that situation and it's better than nothing
The goal has always been to have a minimal codebase. Of course, people should add their own components when building apps with SE-2. And also, we should always be open to adding new ones (or removing / tweaking!).. since this ecosystem is a moving target!
In the particular case of #760:
For me, this is pretty obvious (if we follow the motto of SE-2) that it shouldn't be in the codebase. I mean, why? Same reasoning could lead us to say: "Why not add ScaffoldTable, ScaffoldList, AvatarDisplay, NFTCount, IsOwnerBadge, etc, etc..."
I think we have to draw the line somewhere... and just provide with the things that are obvious. If we are like: "should we add this component?" the answer is probably no.
If #760 was something like ERC20Balance that checks the balance of an ERC20 for a given address, and displays it with some customization... I think that'd be way more inline with everything we are doing. (not saying that we have to do that haha, but that discussion would make more sense)
We don't need to forget that SE-2 is a starter kit for people to use and extend. Adding extra noise won't help. Of course, what is noise or not is subjective and should be open to discussion.... but we should at least be on the same page with the Starter Kit / clean / simple approach.
What do you think?
Really sorry guys for pushing this discussion in wrong direction with shadcn example 😅, lol I should have highlighted that we we should only think about it if we plan to abstract components in the library, which I think is very far in the future.
PS: Also not related to this discussion but there is already a fork of shadcn/ui for web3 https://buidl.turboeth.xyz/docs/
In any case, I think it makes sense to rethink some of the props & display for the components. Maybe having a more standardized way of using the components (sizes, classNames, wrapperClassNames?). Anything that makes the codebase simpler / cleaner + more intuitive to use.
Until we have the components in the codebase itself maybe thinking on this line would be great 🙌
Is it meant to be a full solution from start to finish (building/deploying smart contracts to a fully functional frontend). Or does it act more as a debugging/early prototyping tool where you are expected to take the training wheels off at some point and roll your own frontend using Wagmi/Viem?
I think its a full solution not only for early prototyping but fully prod app. If you consider wagmi/viem in your custom frontend when going prod lol I am not sure what's the reason for ditching SE-2 frontend, SE-2 is just a starter kit which has glued modern web3 stack like wagmi/viem, rainbowkit by default, hence they are always available for you to use it directly, We even have nice recipe how to use wagmi native hook along with all capabilities of SE-2 custom write hook. Regarding the custom hooks and components, we have been mindful about them trying to keep them minimal, not to over abstract keeping the interface similar to wagmi and add things to them which are very necessary / make life a lot easier for general audience.The best thing about SE-2 is we have everything to your expose you can directly go to that component / hook / util and edit according to your need(Things being minimal will also help other experts to go through the code and update to their needs).
The number of unique smart contracts became abundant and the number of hooks became abundant (useScaffoldContractRead(song1), useScaffoldContractRead(song2), useScaffoldContractRead(song3), etc...)
Even if you had used wagmi directly you would have run into same problem of having 28 different hooks maybe viems getContractAt might be useful for this case. Since SE-2 is just a thin wrapper over wagmi, SE-2 inherits this pain point too
I was longing for something like
useScaffoldContractRead(getAllSongs())
Although this is a great idea but again we are trying to keep things minimal and inline with wagmi.
We could create a custom from SE-2 side but again maintaining it and getting it right takes a lot of energy. Also lol getting that right with TS and giving proper type auto completion for return task is huge task with all this dynamic stuff.
^ If this would have been a very common case we could have put in energy and time init...but I think before this Wagmi / viem would have already come up with solution for this.(Since it has alot of audience as compared to SE-2)
It would be nice to do something like useScaffoldContractRead({ contractName: "Song", contractAddress: "0x123", functionName: "tokenURI"}). Where you can override the address, but still reference the same abi.
I have been thinking of this for a while, but if we start allowing people pass contractAddress it will break the multichain #615 stuff which we are doing. Like if people has two chains [chains.sepolia, chains.baseSepolia] and we have same contract deployed on both network, then SE-2 useSCWrite hook will do interal magic and write to correct contract address (depending on user using dapp selected chain), if we allow developer to pass contractAddres then it will not work properly with mutli chain
But really thanks @Hotmanics for thorough response and its always nice to hear people's suggestions. feedback and their pain points 🙌 Just reading the pain points maybe on SE-2 side we could think of :
- If people have same name contract deployed multiple time, how do we generate proper
deployedContract.tsfile for fronenth without ovveriding the previous one. - Maybe think of some way to allow people to pass
contractAddressto hooks without breaking multichain stuff.
hey @Hotmanics
just adding some stuff to Shiv's response.
Or does it act more as a debugging/early prototyping tool where you are expected to take the training wheels off at some point and roll your own frontend using Wagmi/Viem?
I don't understand this question 100%. Not sure what you mean by roll your own frontend using Wagmi/Viem.
Like Shiv said, SE-2 is a starter kit that glues everything together (+ add some nice stuff: burner wallets, debug page, block explorer, some hooks / components / utils + helpers for deployments and some other actions). But the base tech stack is: wagmi, viem, hardhat/foundry, tailwind, daisyUI. You just use anything from that toolbox.
several things that SE2 couldn't properly support.
If SE-2 doesn't support it, it means that nothing from the SE-2 tech stack does.
Another thing is that SE-2 didn't provide a straightforward helper for your use case. This is expected. If we had to add every single possible use case to the codebase, it'd be a mess.
We try to add the most common things, so we abstract some complexity and improve DX.
I think we have to draw the line somewhere... and just provide with the things that are obvious. If we are like: "should we add this component?" the answer is probably no.
So simple and so good 😄 . Agree!
Regarding #760 I think we can close it then
First thanks for all your responses. It's really appreciated and has been a very detailed discussion. All of this will definitely help me hit the target better with future contributions. Apologies if these "principles" are listed elsewhere, I kind of just dove in.
The best thing about SE-2 is we have everything to your expose you can directly go to that component / hook / util and edit according to your need(Things being minimal will also help other experts to go through the code and update to their needs).
The user base is someone familiar with Solidity and React, thus are we assuming they are a pretty skilled developer? If we expect them to go into the source files and change them to fit their needs, then I think we've missed the point of the "minimal" approach. That's just my opinion. I think best case scenario is that they simply import the components and pass in the properties to fit their needs, once they touch the source file then a key principle is lost. I think the hooks follow that really well, however the frontend components do not.
I might need to grasp the concept of Recipes a bit better.
Although this is a great idea but again we are trying to keep things minimal and inline with wagmi.
We could create a custom from SE-2 side but again maintaining it and getting it right takes a lot of energy. Also lol getting that right with TS and giving proper type auto completion for return task is huge task with all this dynamic stuff.
^ If this would have been a very common case we could have put in energy and time init...but I think before this Wagmi / viem would have already come up with solution for this.(Since it has alot of audience as compared to SE-2)
Good points, I understand what you're saying here.
I have been thinking of this for a while, but if we start allowing people pass contractAddress it will break the multichain #615 stuff which we are doing. Like if people has two chains [chains.sepolia, chains.baseSepolia] and we have same contract deployed on both network, then SE-2 useSCWrite hook will do interal magic and write to correct contract address (depending on user using dapp selected chain), if we allow developer to pass contractAddres then it will not work properly with mutli chain
Will try to think on this too. I think that'd be a big upgrade if we can figure a way to make this work properly.
I don't understand this question 100%. Not sure what you mean by roll your own frontend using Wagmi/Viem.
I guess it makes more sense to continue developing in the SE2 project, but you gotta get rid of the SE2 training wheels, and start interacting more diectly with Wagmi/Viem. Regardless, SE2 makes it INCREDIBLY simple to interact with your deployments on the frontend. However, after enough complexity, it seems you gotta ditch it (not the project, rather the built-in hooks, utils, components). I think the more and more we can push that threshold further, the better.
Another thing is that SE-2 didn't provide a straightforward helper for your use case. This is expected. If we had to add every single possible use case to the codebase, it'd be a mess.
We try to add the most common things, so we abstract some complexity and improve DX.
Fair. That seems to be a SE2 philosophy that I need to get aligned on.
@rin-st @damianmarti and thanks for ya'lls contributions to the conversation as well! Sorry if this turned into a long-winded conversation.
In any case, I think it makes sense to rethink some of the props & display for the components. Maybe having a more standardized way of using the components (sizes, classNames, wrapperClassNames?). Anything that makes the codebase simpler / cleaner + more intuitive to use.
I reread the whole conversation and I think this quote is the general conclusion of the discussion.
After more than half a year after we discussed this we (almost) didn't receive requests for changing props and rarely changed them of our components, and hence current se-2 serves users well. I don't mean our components are in the ideal state, but it's pretty good.
Closing this for now. And let's open new issues that will be more specific (maybe after the next redesign?) since quite a lot has been gathered here at the moment, and we don't need most of it.