-
-
Notifications
You must be signed in to change notification settings - Fork 225
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
[refactor] Improve current implementation of atoms #67
Comments
As I was creating components, I found passing down props to Next or Supabase components were a bit annoying... Two things I can mention from the Text component:
1. I'd prefer not to have StrongText componentI'm a bit biased towards not having a "strong text" component mainly because we'll be using more than 2 font-weights across the UI... Eg: Scanning though this GitHub page, I found at least 3 font-weights: We can define the weight as a prop in the component, like we define levels ( It feels like we're hacking Supabase/Next components, when we could create our own with HTML elements.
Do we really need to create a Text component built on top of the Supabase Text component? Wouldn't we be better off creating our own simple component? It feels like we're trying to hack their component for our use... The
|
So I actually deleted the strong text component in the PR related to this issue. Honestly, it was a quick and dirty way of fixing an issue I had with the text component when I wasn't as knowledgeable about supabase ui. So we could build our own ui components but honestly, that'd be a lot more work than it really needs to be and the only thing we really get out of it is a bit more control of each ui component. So for things like text and buttons which are relatively simple to build, I can see why it seems like it makes no sense. But as soon as we get into complicated components, trying to build them out ourselves will take up a massive amount of time that we don't have. Why not leverage something where these components are already built and tweak them to our needs? I think the biggest issue with some of the components we're using now is that we're still relatively new to Tailwind and I'd have no problem applying styles to elements buried in a component but I haven't figured out how to do that with Tailwind yet. Honestly though, I'd much rather hack at a ui library and just make it do what I want rather than build the component myself. There's a lot of states that we may need that we could easily miss trying to build it out ourselves and all of that is just not a great use of the limited time we have. |
That makes perfect sense... Thanks for the thorough explanation, Chad! We can have a chat next week if you want, to think of ways we can use Tailwind on elements buried in components. It might take some Tailwind hacking, but you're right, it'll be faster than creating our own components 🙏 |
Sure, that's no problem at all. Let me know when you want to chat. |
🎉 This issue has been resolved in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Currently out atoms, such as text, strong text and title, are wrappers around supabase ui components. Unfortunately, they don't allow anyone to pass props into the atom and have that prop passed down to the supabase components.
Ideally, the component should allow the props passed into the wrapper to be passed down to the supabase component. This would give more flexibility to the atom and also make it easier to maintain.
The text was updated successfully, but these errors were encountered: