-
Notifications
You must be signed in to change notification settings - Fork 784
Add styled components - Let's talk #480
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
Comments
Honestly, I do not understand how this will help? Can you explain the pros and cons? |
@lex111 well this is what this issue is for. We talked about this briefly on Gitter so I wanted to move this here so everybody can share their opinion. To evaluate if it's worth it to add |
Disclaimer: I've barely used Current StylingPros1. ConciseAll of our styles are already in their respective component file meaning we don't have any extra styling-only files. 2. Explicitly showing stylesThe current implementation is easy to follow because we provide every component that needs a style with a Cons1. Too conciseEverything is in one file. This can be good (see 2. Using
|
This video is a little long (it is actually a workshop) but the talk is given by https://github.com/mxstbr the co-creator of styled-components. Really useful in comparing the different types of styling solutions in react (not focused on react-native but IMO still applies here). The questions at the end are also very good in bring some reasons why CSS styling is good, but how styled-components either has the same features or does it better. https://www.youtube.com/watch?v=ftSzsghg2io This video convinced my team to start using styled-components in our application and it has made our development and testing much more declarative (IMO) |
I don't know if we should be using Currently, every component/screen define its own styles, and it can sometimes leads to inconsistencies (different margins/paddings/sizes). Button were re-designed from scratch every time they were used before rolling up our own Also, with our current implementation, introducing themes (#59) is not doable in a maintainable way. |
Thanks for your feedback guys! I think Also, I think that by using Thoughts? |
@alejandronanez Sounds good! I totally agree with a slow approach on this, not one huge PR. |
@alejandronanez sounds good to me too. I'm just wondering if we will be separating styles from components (i.e have a |
So I've always for some reason thought that because React Native already uses JS for locally scoped component styles as opposed to React for the web, Reading this article and the video you linked @ericadamski - I think I'm sold 👍 @alejandronanez the plan you laid out sounds perfect. Incrementally moving forward updating all of our styles slowly will be nice. Moreover, definitely don't mind including a mention in |
@machour that's a good point. I'm in favour with keeping it within the same file personally to keep the small-components and locally-styled focus the way it currently is. |
@housseindjirdeh another solution we employ is to keep small locally-styled components in a file called |
@ericadamski Thank you, I literally just finished having a chat with someone in work about how to structure things 😂 I'm sorry, I spoke too soon there without being more clear. Keeping things within the same file seems easier for now because that's how I've sort of built each of the components/screen. I agree you're on to something better @ericadamski. Ideally I think having a folder structure for every component might be something we need to move towards. For example:
And our container components (our "screens")
Thinking in this pattern we can use Let me know what everyone thinks as well 🙌 |
Hey @gitpoint/maintainers I'm glad you like the idea of using About the implementation details, it looks a little weird to me to have the style separated from the main component file (since we're not reusing those styles I think we don't need to move them to another file). |
@alejandronanez I think it is a tradeoff. If styles are not reusable and not very long, then we can keep it in the same file. Otherwise, we need separate it out. |
There you go. First styled components PR #495 |
@housseindjirdeh I think that is a great idea! Scoping components and their associated parts (styled, actions, reducers, selectors) helps (IMO) large projects scale. @alejandronanez awesome PR! 🦄 |
Nice thank you folks <3 Okay, let's keep things in the same file for the meantime as that's how things currently are. Opened a separate issue to really start exploring a modular folder structure #506 🕺 |
Quick question about styled components. From what I've seen, you declare Components with its own, encapsulated style. This seems fantastic, but how would we deal with styles and themes across the app? For example, if we want all titles and headers to have the same font and color and nothing else, we could have an easy CSS rule for, say, Same question would go for CSS variables and the such, how do we address those? |
@SammyIsra the idea behind component architecture and therefore styled components is that creating many small isolated chunks of a system (in the case of styled a frontend UI) and reusing the small isolated chunks allows consistency and simplicity when building large applications. Your suggestion for a CSS rule like For example, consider a page header that contains a title, a sub-title and some container elements to allow some funky styling. Using class names as your component driver you will have something that looks like this: (NOTE: this is not react native but react, the same ideas apply to both) const Header = () => {
return (
<div className="header">
<div className="header--inner">
<div className="header--title--container">
<h1 className="header--title">Title</h1>
<span className="header--title__sub">Sub Title</span>
</div>
</div>
</div>
)
} If you contrast above with a styled components approach and notice the more declarative nature of the render method: import { Container, Inner, Title, SubTitle } from 'styled.js';
const Header = () => {
return (
<Container>
<Inner>
<Title>Title</Title>
<SubTitle>SubTitle</SubTitle>
</Inner>
</Container>
)
} Take Beyond that, say you have to change the colour of the title if it is used as a primary title or a secondary title, this would involve 2 extra classes that override the parent style and now we have to consider collisions and silly CSS precedence. With styled components you can just pass props and use JS logic to apply it directly in the component, for example: const Title = styled.h1`
display: block;
font-size: 2rem;
color: ${props => {
if (props.primary) return 'red';
if (props.secondary) return 'blue';
return 'white'; // default
}};
`; // the simple styled component
// in use
<Title primary>Primary Title</Title>
<Title secondary> Secondary Title</Title> Since this is all jS managing CSS variables becomes just managing JS variables like having a colours object that has the specified global colours: const colors = {
black: '#000',
};
// and we can use it in a styled component like
const Title = styled.h1`
color: ${colors.black};
`; The power of styled components matches if not exceeds that of plain CSS or even SASS/LESS and allows a more declarative style of writing react. |
Fantastic explanation, thank you! @ericadamski One last question: @andrewda mentioned "Syntax highlighting" as a pro for |
@SammyIsra @andrewda is talking about code editor syntax highlighting, currently the styling follows a JSON structure so it cannot syntax highlight for CSS, but styled components you write actual pure CSS in JS so there is CSS style code syntax highlighting. It just makes writing the code a little more comfortable. :) |
Since we all decided that this is the way to go, I'm going to lock down this issue. If you think it's still worth discussing anything else, feel free to unlock it. |
Add styled components - PoC
Update
Let's talk about what is "wrong" / "right" about our current implementation for styling, and what styled-components will bring to the table. If we're not going to see any real benefit I think we should hold this off.
Update 2
There's a list here #532 with all the components that need styling, feel free to grab the component you want to work on.
The text was updated successfully, but these errors were encountered: