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

Refactor Layout CSS Classes to improve maintainability and avoid duplication #415

Open
camposcristian opened this issue May 4, 2020 · 1 comment

Comments

@camposcristian
Copy link
Contributor

camposcristian commented May 4, 2020

Original Question:

@davidpaley I can change it and put this into the css module file, but how should we actually refactor this? There's like 4 references to the same class in different files and I suspect there will be more. Is there any point on keep repeating ourselves?

image

Answer from @mischacolley :

@camposcristianeze what you are looking at there to me is a classic case of a utility class. Or something that should be. And I'd traditionally think about this as a layout utility because layout is what it's trying to effect.

It's actually fascinating to see we've gone from wrap mx-auto px3 to full-width-wrap wrapping up three utilities from https://basscss.com/ into one class that we are trying to now understand how to use in a CSS Modules context...

Without having a really clear picture of this code I'd assume that these classes are doing more than just setting width: 100% (i.e. they are doing this with margin, padding etc too) and they are in fact enforcing whatever layout the design has dictated. Having these reusable layout utilities is previously how I would have maintained consistency across "pages" referencing these styles from one file.

Thats not whats happening here as you I think are alluding to. We've essentially got a group of classes that sound like they do more or less the same thing authored across multiple files. The duplication you refer to.

Pre Components and CSS Modules I would have just refactored these to be consistently named with a consistent visual function and put them in a global utilities css file so they can be used wherever needed. This is where things like https://tailwindcss.com/ have grown from as people realised you could take that idea a lot further.

Now with CSS Modules you face a conundrum. Where should these styles go and how many times will you write the same style rule in each component? In this case let's assume you want to make something full width (whatever that means) in a number of different contexts .... how do you do it?

Originally posted by in https://github.com/_render_node/MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDQxOTIwNzc1MQ==/comments/review_comment

@camposcristian
Copy link
Contributor Author

@mischacolley I think once we tackle this we should be far more quicker on delivering changes as the structure won't change considerably. Plus we'll improve performance considerably.

Now with CSS Modules you face a conundrum. Where should these styles go and how many times will you write the same style rule in each component? In this case let's assume you want to make something full width (whatever that means) in a number of different contexts .... how do you do it?

Even with css-modules we're already using @import "../../styles/base/variables"; across over 130+ css files. I think we could have a similar layout base where we don't have to worry about it? Otherwise, there are tons of different approaches here --> https://nextjs.org/docs/basic-features/built-in-css-support

But at the end of the day, everything depends on the longer term picture. I think we got a nice structure now that we'll try to update and maintain for the next years, so let's look at something we know it has a bit of future going forward..

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

No branches or pull requests

3 participants