styled-components / comparison

Comparing different ways to style components
MIT License
183 stars 23 forks source link

CSS Modules example #10

Closed MicheleBertoli closed 7 years ago

MicheleBertoli commented 7 years ago

Hello everyone!

This is the first iteration on the CSS Modules example. I implemented the Tweet Card from mobile.twitter.com, using the following tweet (as agreed in #3): https://mobile.twitter.com/mxstbr/status/755481795206971392

Notes:

I hope you like it and I'm looking forward to receiving some feedback.

359px

359

360px

360

600px

600

mxstbr commented 7 years ago

On the road right now, I love that you included listing. How strict is it? We need to be as strict as humanly possible.

mxstbr commented 7 years ago

Maybe we should go with vanilla airbnb? That'd make it easy for most people.

mxstbr commented 7 years ago

Finally getting around to checking this out now, sorry for the wait! Let's kick this mofo off!

mxstbr commented 7 years ago
  1. Let's not render into document.body, let's choose a root node. (something unique like #example-root) This will make creating the site with the different approaches a lot easier when we get around to that.
  2. I still want to use vanilla airbnb. It's not about the whole semicolons-or-not thing, I think that it will make people more comfortable with the style if we use the vanilla one.
  3. I like the way you handled the whole data thing. Maybe the transformation utils and the data should be in a shared, top-level folder so all the examples can use the same functions and data without copy and pasting?
  4. Same goes for the eslint config, that should be shared example-wide?
  5. What about the assets? Shouldn't they also be shared?
  6. Use http://mxstbr.com/headshot.jpeg for the tweet avatar โ€“ I changed my avatar and now the display is broken, which shouldn't happen. mxstbr.com/headshot.jpeg will always exist ๐Ÿ‘

Other than that, the component structure and all that looks good to me!

MicheleBertoli commented 7 years ago

hey @mxstbr, thanks for reviewing my PR. I'll push a new commit with the changes.

I've got just one question:

I still want to use vanilla airbnb. It's not about the whole semicolons-or-not thing, I think that it will make people more comfortable with the style if we use the vanilla one.

do you want me to remove only the "rules" or do you think it's better removing the "import/resolver" setting too?

sharing data, config and assets sounds great. I think we should merge this one first and then we can create a shared folder in a different PR, if it makes sense for you.

mxstbr commented 7 years ago

I think we should merge this one first and then we can create a shared folder in a different PR, if it makes sense for you.

Sure thing, sounds good to me โ€“ let's get this done!

do you want me to remove only the "rules" or do you think it's better removing the "import/resolver" setting too?

Just the rules should be fine, I think. ๐Ÿ˜Š

MicheleBertoli commented 7 years ago

It should be ok now @mxstbr. Please let me know if there's something else I can do to improve it.

mxstbr commented 7 years ago

I like it. I say we merge it, factor out the common stuff and get onto the next one! ๐Ÿ‘

What do you think @geelen @JedWatson?

mxstbr commented 7 years ago

Just realised we're including a .ejs template without any ejs specific stuff. Let's rename it to index.html, throw it in the root of the example and point HTMLWebpackPlugin at that?

Seems more obvious for beginners.

EDIT: Doing this now, will then merge and extract common stuff!

mxstbr commented 7 years ago

I also don't think we need to differentiate between containers and components โ€“ย really, they're all components. Let's put them in the same folder in an effort to put the focus on the method of styling used!

mxstbr commented 7 years ago

Along the same vein, what do you think about getting rid of the resolve.root setting? I feel like this'll only serve confuse newcomers not used to webpack, I'd much rather avoid that and have the focus be on the styling method used.

mxstbr commented 7 years ago

We should also add a README to all methods of styling used and explain where to find the docs and what some of the specialities and ideas are โ€“ย I can do that.