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

Should be able to append CSS classes #264

Closed
rikusen0335 opened this issue Jan 13, 2021 · 10 comments
Closed

Should be able to append CSS classes #264

rikusen0335 opened this issue Jan 13, 2021 · 10 comments
Assignees
Labels

Comments

@rikusen0335
Copy link

This issue is a:

  • Feature request

Description

Currently you can add CSS styles with wrapStyle prop, but we need to add css through string like:

<Zoom className={classes}>
  <img something={something} />
<Zoom>

Because there's some cases to use CSS Modules or utility first css frameworks like Tailwind CSS.

@rpearce
Copy link
Owner

rpearce commented Jan 15, 2021

@rikusen0335 Thank you for your feature request idea contribution to the project!

There is another major rewrite underway that will change how the component is consumed, and it will allow you to do what you require with styling.

I don't know when version 5 will be available, and I will keep this issue open until it is resolved.

Thanks again.

@rpearce rpearce self-assigned this Jan 15, 2021
@rikusen0335
Copy link
Author

Thank you for answering! I am looking forward to it 🥇

@rpearce
Copy link
Owner

rpearce commented Mar 28, 2021

Update on v5 FYI: #274

@rpearce
Copy link
Owner

rpearce commented Jun 14, 2022

@rikusen0335 I'm working on things and am unsure how I want to go about this. At present, you can override/add on to the default styles, but passing a single className — now, as well as for the next version — raises some questions.

  • What would you be looking to achieve that you couldn't by targeting .my-class [data-rmiz-wrap] { } in your CSS?
  • Should there be a className prop be added to every element, including all of the modal ones? (This feels incorrect to me)

I know it's been a while, and thank you for your time.


Here's the latest update on the work being actively done! #274 (reply in thread)

@rikusen0335
Copy link
Author

You probably need to override classes into every element, like if you making the lib, I mean like:

const Zoom = ({ baseClasses, secondClasses, src }) => {
  return (
    <div className={`base ${baseClasses}`}>
      <figure className={`second ${secondClasses}`}>
        <img src={src} />
      </figure>
    </div>
  )
}

So, because recent CSS-in-JS stuffs are bit difficult to use css selectors.

@rpearce
Copy link
Owner

rpearce commented Jun 14, 2022

That seems like potentially a lot of classes to accept and use; in the v5-dev branch code, I would need to accept 9 different class names to cover all the elements.

Don't all the CSS-in-JS provide options to break out of their modules? E.g.,

Using the example from https://emotion.sh/docs/nested, here's how you would target data-rmiz from the v5-dev branch:

const myTextWithNestedImg = css`
  color: purple;
  [data-rmiz] { /* some styles here */ }
`

Or going with the global approach from emotion:

<Global
  styles={css`
    [data-rmiz] { /* some styles here */ }
    [data-rmiz-content] { /* some styles here */ }
  `}
/>

@rikusen0335
Copy link
Author

rikusen0335 commented Jun 15, 2022

Ohh I didn't expect how that way can be done. That seems great, so I'd choose css selector way now.

@rpearce rpearce closed this as completed Jun 16, 2022
@rpearce
Copy link
Owner

rpearce commented Jun 16, 2022

Thank you for asking your question and communicating in hopes of making this better! I've added you to the README's all contributors list:

imagen

@rikusen0335
Copy link
Author

I even didn't anything xD
But thank you!

@rpearce
Copy link
Owner

rpearce commented Jun 17, 2022

You asked a question and thought through some ideas with the hopes of making the project better! That is a contribution ❤️

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

No branches or pull requests

2 participants