-
Notifications
You must be signed in to change notification settings - Fork 867
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
Make components more customizable #786
Comments
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 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 #733 kind of adding I think Props like Maybe in case in future if we plan to abstract components in library then we might need to go with approach of So instead of having only <Alert>
<AlertTitle>Heads up!</AlertTitle>
<AlertDescription>
You can add components to your app using the cli.
</AlertDescription>
</Alert> This way you can pass 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: 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. SolutionsScaffoldAddress/BaseAddressI’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
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...
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 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, but sometimes you need the same component with different styles or behaviors, so having some customization available in the component is useful.
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. |
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! |
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 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!
Copying from a previous comment, this could be a first good iteration:
|
Had the similar thoughts reviewing last pr's.
I had experience with that kind of props, and every time it became unmaintainable chaos, so I'd want to avoid it.
<Alert>
<AlertTitle>Heads up!</AlertTitle>
<AlertDescription>
You can add components to your app using the cli.
</AlertDescription>
</Alert> I also have experience working with I'll review the components next days and try to think about possible solutions. Yes, I think we need to standardize props. Or use
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):
|
Not taken personally at all! I came running head first, when I should have gathered context beforehand. So I do apologize for that.
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 1Originally, in my deploy script (you can see it commented out), I had deployed 28 instances of the My initial solution was to do something like Luckily, I did have a Part 2For some unrelated reason (I think gas costs), I had to switch my whole operation to create and include Some loose thoughtsThis 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 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? |
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:
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 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 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/
Until we have the components in the codebase itself maybe thinking on this line would be great 🙌
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).
Even if you had used wagmi directly you would have run into same problem of having 28 different hooks maybe viems
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)
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 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 :
|
hey @Hotmanics just adding some stuff to Shiv's response.
I don't understand this question 100%. Not sure what you mean by 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.
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. |
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 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.
Good points, I understand what you're saying here.
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 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.
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. |
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.
The text was updated successfully, but these errors were encountered: