search

LottieFiles / lottie-react

lottie web player as a react component
https://lottiefiles.com
MIT License
720 stars 80 forks source link

Consider attaching `className` to the outer-most container #103

Open kdembler opened 2 years ago

kdembler commented 2 years ago

Currently, even though the Player accepts a className prop, it is not passed to the outer-most container of the animation. So the render tree will always be .lf-player-container > #lottie and only #lottie can be styled via className. This creates some issues when you need to style the most direct container of your views for layout purposes etc. Solutions that I would consider for this issue:

  1. Attach className to .lf-player-container instead
  2. Remove .lf-player-container altogether and make #lottie the direct container
  3. Add some additional prop that allows styling .lf-player-container.

At the moment the only way to style the .lf-player-container is to use some direct descendant selector or use the class name directly, but then we need to leak internal implementation of the library into our codebase which we consider a bad practice.

Thanks for considering and thanks for the great library! :)

samuelOsborne commented 2 years ago

hi @kdembler

Have you tried overriding .lf-player-container with your own CSS?

It doesn't have any styling on our end, and was left for user's to add their own.

kdembler commented 2 years ago

Yes, we tried that. The issue is that like I said we would like to avoid having styles based on the .lf-player-container class in our codebase as we may want to switch to a different lottie library in the future without things breaking (we just switched to this one :)). So like I said, ideally, the internal implementation of the library (container class name) wouldn't need to leak inside our codebase

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.

LifeLike1 commented 1 year ago

Would it be possible to recheck this?

One of the solution is to replace <div className="lf-player-container"> with React Fragment <> so it doesn't create div without any styles and usages. It's pretty simple to add existing container by yourself.

Another approach would be adding props for styling lf-player-container as @kdembler mentioned

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.