Skip to content
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

Feature Request: Unstyled Buttons #1116

Open
mykcryptodev opened this issue Aug 20, 2024 · 9 comments
Open

Feature Request: Unstyled Buttons #1116

mykcryptodev opened this issue Aug 20, 2024 · 9 comments

Comments

@mykcryptodev
Copy link
Contributor

Describe the solution you'd like

Allow me to pass a prop to a button like TransactionButton that removes all styling so that I can be sure that my classNames are the only styles applied.

Describe alternatives you've considered.

I've tried fiddling with classNames and using !important to try to get my custom styles to be applied as desired but it would be much easier if I could tell onchainkit to be styleless

@0xGurmy
Copy link

0xGurmy commented Aug 24, 2024

I was able to avoid using !important to fully style the button by targeting the span inside the buttons, but this took me a bit of playing around before it became evident that I needed to target the span within the wallet button.

I don't know if it's possible, but it would be really nice if I could style everything with the button class, without the span, because I have my buttons styled in my MaterialUI theme.

Adding here because I don't know if this deserves it's own issue 🫠

@Zizzamia
Copy link
Contributor

Zizzamia commented Sep 3, 2024

@mykcryptodev and @0xGurmy can you share code example of your experience. I have a few ideas on how to solve this, but i would like to get more info on your experience first.

@mykcryptodev
Copy link
Contributor Author

@Zizzamia here is an example that I whipped up

Here are two buttons where I provide the same style (btn btn-primary).
My component looks the way I want it to look
The button updates the background color correctly but the text is not styled as intended.

Screenshot 2024-09-04 at 10 56 46 PM

if I look at the elements that are being created for the connect wallet button, there is a span within the button that styles text size, text color and tracking of the button label.

Screenshot 2024-09-04 at 11 00 04 PM

Another solution could be limiting nested tags within components that override classes passed in from props.

Here is the example app branch
Code for wallet component
Code for sign in component
Demo page

@Zizzamia
Copy link
Contributor

Zizzamia commented Sep 8, 2024

@mykcryptodev so to give more control on this particular case, we are going to explore a new component added to the Wallet family, <ConnectWalletText>.

Here the PR #1222

You are going to use it by doing

<ConnectWallet>
  <ConnectWalletText>Connect Wallet 🌊</ConnectWalletText>
  <Avatar address={address} className="h-6 w-6" />
  <Name />
</ConnectWallet>

This is not a perfect solution, but for sure is a step forward to the text prop that was not allowing you custom style.

More I think about this, and more I am realizing that the text prop in general is a bad idea, and whenever we use it we should remove it.

For now we are not deprecating the text prop yet, but it's something in coming releases we will. Probably in 5~6 weeks, so we give time to the community to adjust to using <ConnectWalletText>.

Oh, and I am going to close this Issue after we land this in the next version and I will have updated the docs.

@mykcryptodev
Copy link
Contributor Author

Understood. thanks for considering this! So it sounds like the prevailing idea is to give the developer control over each nested element within an onchainkit component.

In this example, the span element that styles the connect button text can be styled by including the component and applying classes there.

It sounds like if this pattern continues throughout onchainkit, there would be a world where if I wanted to style the text of a , there would be a component? Is that the idea?

@Zizzamia
Copy link
Contributor

Zizzamia commented Sep 9, 2024

Yeah, at least for now seems the most balanced way.

But, it's fair to stay vigilant on this design decision and keep re-evaluate that overtime.

@mykcryptodev
Copy link
Contributor Author

Here is different example that falls into the same bucket where onchainkit's styling throws me off

Here I want to have my onchainkit transaction button sitting next to a different button that is not from onchain kit.
I use flex to put them next to eachother and I use items-center to vertically align them however you'll notice that my onchainkit button is not aligned
Screenshot 2024-09-13 at 9 32 57 AM

this is due to onchainkit adding margin above the button in it's own styling
Screenshot 2024-09-13 at 9 35 03 AM

Unfortunately, I cannot simply pass in a tailwind class to remove the top margin on the transaction button because onchainkit's margin is on the button element within the div that I am applying classes on
Screenshot 2024-09-13 at 9 35 45 AM

I would need to write my own custom class to handle this situation which is not ideal

@Zizzamia
Copy link
Contributor

Zizzamia commented Sep 14, 2024

@mykcryptodev great call out, but here 2 points:

Point 1
You can control directly the style with <TransactionButton />, and do

<TransactionButton className="mt-0" />

and it will work, no? That's the all point of having sub components you can directly control.

Point 2
Btw, a general point about margin, that I just realized, is that for any of the primary component we should allow margin style within OnchainKit. That's because primary components should not modify the space around them, but only within them. Anyway removed the mt-4 with this PR. #1258

@mykcryptodev
Copy link
Contributor Author

You can control directly the style with

@Zizzamia that does work. I have to get used to the pattern of having sub-components. Thanks for the tip

primary components should not modify the space around them

Agreed. Thanks for that update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants