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

Design system #88

Open
travi opened this issue Dec 11, 2017 · 8 comments
Open

Design system #88

travi opened this issue Dec 11, 2017 · 8 comments

Comments

@travi
Copy link
Member

travi commented Dec 11, 2017

https://github.com/jxnblk/styled-system

@micleners
Copy link
Member

In their documentation that use styled components. In their code sandbox for emotion they use styled from @emotion/styled.

Perhaps we should revisit the conversation around using styled components. Using styled components is my preference, rather than css objects assigned from the css prop @emotion/core

@travi
Copy link
Member Author

travi commented Oct 27, 2019

from https://styled-system.com/how-it-works ;) :

By using style objects instead of embedded CSS strings, Styled System is compatible with a wide range of CSS-in-JS libraries.

honestly, neither the object approach nor styled components should be limiting. a big part of the object approach for me is that it just lends itself more easily in my mind for importing from other files and manipulation with js. for example, we haven't really used it here yet, but i've found polished to be really helpful to bring a bunch of the sass functionality to css-in-js and it just seems like a more natural fit for use with the object approach to me. i'm sure it could work with styled as well, but i just have a harder time getting my head around that way.

is there a big roadblock that youre seeing with the object approach, or mostly the overhead of trying to learn both?

@micleners
Copy link
Member

We seem to be having a misunderstanding. Originally, it sounded like css objects vs styled components but the link you shared and the sandbox I sent above show that style objects are compatible with styled components. So why not use both? Here's an example - how I would have approached changing the social items styling (instead of creating a div with a css prop):

micleners@a377145

In the example I install @emotion/styled. I take the style object that i used in my social links PR, and create a styled component with them.

The current approach of assigning css with the css prop feels a lot like inline styling - which I only care for with prototyping styles. Styled components offers reusability and as the documentation you are sending references it, it seems to be a preferred approach. If it works with style objects so we can avoid CSS strings, is there reasons not to use styled components as well?

@travi
Copy link
Member Author

travi commented Oct 27, 2019

Here's an example - how I would have approached changing the social items styling (instead of creating a div with a css prop)

i think this might be part of where i'm not explaining myself very well. my goal is to avoid the extra DOM element that is only being added to enable styling, whether that is a <div> that we style with the css prop or a styled component. i may be misspeaking on the details of styled components since i am not intimately familiar, but i'm fairly sure that an extra DOM element is created in that case.

instead, i'd like to style the ones that already have to exist because they have semantic meaning. in this case, we already have a DOM element that acts as a container for the icon and text, the <a> that is encapsulated inside the <ExternalLink>. that encapsulation does complicate things slightly, so the open question that i was trying to highlight was whether we style the <a> by passing a css prop to <ExternalLink>, which is in-turn passed to the <a>, or if we instead define a variant of the <ExternalLink> that handles the alignment of the icon with the text. if we did the latter, the key is to do it in a way that can be more generic than this particular context, but i think a link with an icon and text is likely to be a pretty generic link variant. does that make sense?

as far as the open question goes, i think i've convinced myself through the <InternalLink> improvements that the variant approach is worth the effort. i'm happy to talk through what it would take in more detail, but i think thats the way i'd like to see us take this unless theres a strong reason not to.

So why not use both?

with the explanation above, would you still see value in having both approaches used? i see value to having a consistent single way to apply styles, but could be missing part of what youre trying to show. if the options are between passing an object to the css prop of an <a> vs creating a different <a> using styled components, that seems like simply two ways to accomplish the same thing, but am open to new information that i might be misunderstanding.

feels a lot like inline styling

while the implementation starts out that way, its not how the library applies the styles, so it doesnt come with the downsides. as we've already shown, though, the rules can easily be pulled out to a variable or even a separate file. the links are already an example of needing to do that in order to share the same rules across multiple components. whether the rules are defined inline in the jsx is an implementation detail in my mind, but the flexibility to pull out further and further is a strong benefit, imho.

@travi
Copy link
Member Author

travi commented Oct 27, 2019

for a little background on my preference of using minimal, semantic-markup, the idea of divitis highlights a big part of what im trying to communicate. once you eliminate extra DOM elements that are only there for style, i think the difference between the css prop on a real DOM element vs a styled component is just implementation details for the same outcome. if that's the case, having two ways to do the same thing seems like just asking for confusion from new contributors

@micleners
Copy link
Member

It sounds like the answers here come down to a combination of philosophy and principle. You are probably correct that each approach on it's own is valid and can achieve desired goals. I like the push to minimize the amount of extra elements on the DOM. Also, ease of contribution for a newcomer is a great focus. How to accomplish this though, is a great question.

From what I've seen, styled components allows you to organize and reuse your components in such a way that it reduces the amount of noise in the dom, including extra elements. To replace a div with some styled you don't need to wrap in in a styled component, you can just replace is with a styled component. Styled components can also extend components, allowing for simple CSS mutation. It also allows for styles applied from props.

As far as a newcomer coming in, having to adhere to one approach might be more challenging or confusing. I understand the consistency within the codebase argument. I think the styled components approach creates a cleaner separation of styles and element arrangement (rather than inline styles via props) that yields a more readable and easier approach to the codebase.

I'm keen on learning a variety of approaches, including the one you are advocating for here. Maybe it is just a difference in approach in the end - no real benefits with either approach. I don't know if I have experienced enough to really say that, but the example you are giving here in this issue does support the use of styled components.

@travi
Copy link
Member Author

travi commented Oct 28, 2019

As far as a newcomer coming in, having to adhere to one approach might be more challenging or confusing. I understand the consistency within the codebase argument.

i think this is the unfortunate state of styling with react at this point. there are many choices out there between raw css, css-modules, sass with css-modules, before even getting into the css-in-js options. it does come with a need for a bit of hand-holding with newcomers, but i think that is part of being a welcoming project anyway. i'm already planning to capture some of the decisions that have been made as i catch up on some more of the contribution docs, so im hoping that will help some too.

I think the styled components approach creates a cleaner separation of styles and element arrangement (rather than inline styles via props) that yields a more readable and easier approach to the codebase.

I'm keen on learning a variety of approaches, including the one you are advocating for here. Maybe it is just a difference in approach in the end - no real benefits with either approach. I don't know if I have experienced enough to really say that, but the example you are giving here in this issue does support the use of styled components.

since this thread has gotten pretty distracted from the original issue, we should probably either open a new design system ticket and rename this one or move further conversation to somewhere like slack. i'm more than happy to keep discussing and am always option to learning pieces that i may be overlooking/misunderstanding. it does seem to me like two options of accomplishing the same goal, but i dont want my understanding to shut down the conversation or make it harder for you to continue contributing :).

@micleners
Copy link
Member

Cool. I agree - let's table the discussion for now. Maybe we can also get folks to weigh in who have more experience on the future of the design system for the site (or just wait until the potential redesign to do that and just continue forging forward with your plan)

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

No branches or pull requests

2 participants