search

alivenotions / pidpod

A friendly podcast player
MIT License
0 stars 0 forks source link

Move to styled components for styling #5

Closed alivenotions closed 5 years ago

alivenotions commented 6 years ago

Using styled components will help us to divide styles also in components and make it easier to switch styles on runtime (for themes etc.), at least that is claimed by the author. But I don't see how are they any better than plain old CSS.

gbhavalkar commented 6 years ago

I think the scope of the css defined is only for the component itself as the generated classNames will be different per component and so there will be minimal css side-effects (not sure if there will be any). However the css properties defined in the styled components should not be passed down through any parent, as changing some property in the parent will have an effect on the child. For eg: applying theme throughout the app, only the theme name should be passed as the prop and not the properties itself. But this means there is a need to define multiple properties according to the theme passed down (and the lazy in me thinks there has to be a better way to do this).

alivenotions commented 6 years ago

I think the scope of the css defined is only for the component itself as the generated classNames will be different per component and so there will be minimal css side-effects (not sure if there will be any)

That is true. But debugging it can become a nightmare as the class names are garbled rubbish.

applying theme throughout the app, only the theme name should be passed as the prop and not the properties itself. But this means there is a need to define multiple properties according to the theme passed down (and the lazy in me thinks there has to be a better way to do this).

I don't think we need to do this. The theming part of styled-components allows you to create a theme and pass it down using the context API. The whole performance considerations should be for the context API itself. And I don't think that all the children will be re-rendered.

alivenotions commented 5 years ago

@gbhavalkar Maybe we should have this? Facing major issues in theming.

gbhavalkar commented 5 years ago

We can. Although need to research how does the theme passed through the ThemeProvider component interact or coordinate with the individual styling. Also is it a bad practice to maintain the theme styles in a centralized way for all the components to access (kind of like a mini-bootstrap). One issue with this i feel is it will nullify the effect to css chunking. But yeah we can certainly use this as i feel as though we will end up implementing the same way by passing the style to child components

alivenotions commented 5 years ago

it will nullify the effect to css chunking

How much of an overhead can that realistically have?

gbhavalkar commented 5 years ago

Man testing with styled components is quite a pain!!