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

All components need an optional "responsive" prop #97

Open
benjitrosch opened this issue Apr 19, 2022 · 1 comment
Open

All components need an optional "responsive" prop #97

benjitrosch opened this issue Apr 19, 2022 · 1 comment
Labels
good first issue Good for newcomers

Comments

@benjitrosch
Copy link
Collaborator

benjitrosch commented Apr 19, 2022

For built-in responsiveness we need to add an optional "responsive" prop (defaulted to true in Storybook) pre-baked with TailwindCSS breakpoints:

https://tailwindcss.com/docs/screens

Ultimately, users should implement their own sizing to suit their needs. However, at the moment many of our Storybook components are too large for the preview container on mobile and this doesn't give a good first impression for the library. Additionally, some users might not want to implement responsiveness themselves and would rather use a general purpose default option.

Adding the prop would look something like this:

  1. Add responsive?: boolean to the IComponentBaseProps interface in types.ts (https://github.com/daisyui/react-daisyui/blob/main/src/types.ts) to make the prop available on all react-daisyUI components.
  2. Include the responsive prop name when destructuring the component's Props type in it's definition
  3. Using the clsx library, add sizing for the sm, md, and lg TailwindCSS breakpoints (or wherever applicable), set conditionally when responsive equals true. For example:
clsx({
    'm-2 sm:m-4 md:m-8 lg:m-16': responsive,
})
@benjitrosch benjitrosch added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 19, 2022
@benjitrosch benjitrosch removed the help wanted Extra attention is needed label Jul 26, 2022
@mazondo
Copy link

mazondo commented Aug 22, 2022

Instead of adding responsive specifically to start, thoughts on just adding first class support for className in ways that make sense for each component? I'm hitting that right now with the Dropdown component - you cannot pass custom classes into it and as a result things like a full width button trigger are a pain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants