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

Create custom style prop or more prop to customize style #79

Closed
emewjin opened this issue May 3, 2022 · 8 comments
Closed

Create custom style prop or more prop to customize style #79

emewjin opened this issue May 3, 2022 · 8 comments
Labels
🚀 enhancement New feature or request

Comments

@emewjin
Copy link

emewjin commented May 3, 2022

Is your feature request related to a problem? Please describe.
I add w-full class on Button component to use it by full width, but internal class w-fit ignore it.

 <Button type='button' className='w-full bg-primary-500' onClick={handleOpenAddView}>
   글 작성하기
 </Button>

image

Describe the solution you'd like
maybe 2 option (just suggestion). Also, I think other components will need this option too.

  1. add width prop on Button component. then user can customize button component's width
// for full width button
 <Button width='full'>
   sdfsdf
 </Button>

// for CTA button 🟩🟩🟩  🟦
<Button width='0.75'>
  🟩🟩🟩  
</Button>
<Button width='0.25'> 
  🟦
<Button>
  1. add custom style prop to customize free without priority issue
 <Button custom='w-full'>
   custom prop should have high priority then internal class
 </Button>

Additional context
discord thread

@emewjin emewjin changed the title Create custom style prop or more prop Create custom style prop or more prop to customize style May 3, 2022
@tulup-conner
Copy link
Collaborator

I think there's too many possibilities to add options as unique props, but we definitely need an "escape hatch" to override className. As you suggested, custom.

After you brought that up on Discord, I found this: https://github.com/richardgill/tailwind-override which I think we can use to add that escape hatch. That would make it so className can be provided to each component, and any classes will be applied without needing !important.

@rluders rluders added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels May 3, 2022
@emewjin
Copy link
Author

emewjin commented May 3, 2022

@tulup-conner how about this? https://github.com/ben-rogerson/twin.macro

tailwind-override doesn't seem to be updated anymore

@tulup-conner
Copy link
Collaborator

tulup-conner commented May 3, 2022

@tulup-conner how about this? https://github.com/ben-rogerson/twin.macro

tailwind-override doesn't seem to be updated anymore

Yeah, tailwind-override isn't the best option for that reason and because it seems to have some outstanding unfixed bugs. They also chose to use regex to parse class names which inflated the bundle size.

Twin brings in a lot of dependencies and unrelated functionality. The dependencies thing is something we really want to avoid.

This isn't really my area of expertise, so I'm doing the same thing you are, looking around for libraries that have done it right already. The actual maintainers of this project are actively trying to solve this as well so I'm sure we'll get there.

@rluders rluders added the 🚀 enhancement New feature or request label May 4, 2022
@tulup-conner
Copy link
Collaborator

tulup-conner commented May 26, 2022

Following up, we are starting to complete theme support for components now; see #151, #153, #154, #155.

Note: The API is unstable (almost certain to change in some details before "done")

Your use case will be solved here in general, and I will make sure you can change this class in particular 💯

@tulup-conner
Copy link
Collaborator

tulup-conner commented May 27, 2022

See #160, it's in draft right now.

It just occurred to me that there are probably situations where you might want to add a className in a subset of your buttons, but not all of them. Brainstorming..

@emewjin
Copy link
Author

emewjin commented May 28, 2022

See #160, it's in draft right now.

It just occurred to me that there are probably situations where you might want to add a className in a subset of your buttons, but not all of them. Brainstorming..

So cool !!
If I want to use Button w-full only in specific component, Is this what I should be doing?

const theme = {
  button: {
    base: 'w-full',
  },
};

or should like this?

const theme = {
  button: {
    base:  'group flex h-min w-full items-center justify-center p-0.5 text-center font-medium focus:z-10',
  },
};

@rluders
Copy link
Collaborator

rluders commented May 28, 2022

@emewjin the second option. 'Cause you will always complete overwrite the style that you are customizing on your own theme.

@tulup-conner
Copy link
Collaborator

We should document that behavior for sure ^

@rluders rluders closed this as completed Jun 2, 2022
@rluders rluders added this to the v1.0.0-alpha milestone Jun 2, 2022
@rluders rluders moved this from Backlog to Done in Flowbite React Development Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

3 participants