Migrating project back to CSS Modules with SCSS from style-components#239
Migrating project back to CSS Modules with SCSS from style-components#239
Conversation
✅ Deploy Preview for webdevpathstage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
oluwatobiss
left a comment
There was a problem hiding this comment.
Great work here, @cherylli! I'm curious about the Index naming choice. Please see the inline comment.
Satoshi-Sh
left a comment
There was a problem hiding this comment.
Looks good.
Is it better to have a list to track which part we completed the migration on #238 or somewhere?
Blog
- AuthorBio
- BlogPostContainer
ContactUs
- ContactUsCards
- ContactUsForm
Yeah i'm thinking to something like this in a channel canvas on slack |
briangesteban
left a comment
There was a problem hiding this comment.
This looks good already. Just a suggestion tho, maybe we could modularize the partials, like instead of having all variables in _variables.scss we could put it as _colors.scss, _breakpoints.scss, _typgraphy.scss, etc. Same with mixins, it could be like _media-queries.scss, etc. and also adding fallback fonts. Other than that everything looks great to me.
I'm not sure if putting colors in scss will work for theming since it's compiled at build time, thats why I have them as normal css variables, but feel free to test it and let us know |
Yes, you are absolutely right but there's a work around for it that we could do. We could implement a CSS/SCSS variables hybrid. Theming will still use the CSS variables, then we will use interpolated SCSS variables as a value for those CSS variables. SASS have an interpolation method, which looks like this Colors Partial File Theme Partial File |
|
@briangesteban sounds good, feel free to make a PR for that |
Have you updated the CHANGELOG.md file? If not, please do it.
Yes
What is this change?
The main purpose of this change is to test out if css modules and styled-components can co-exist, and migrate fully to CSS modules in stages.
In this PR,
ContactUsCardsis changed to using CSS modulesstyles/themes.scss)CSS variables are used for theming mainly due to the fact that it can be changed during runtime, unlike scss varibles, theme is set during compile time.
To change theme we would set the data-theme attribute on the document like so
document.documentElement.setAttribute('data-theme', 'dark');for non default themes
I also want to use a different file structure — specifically, placing all style-related files close to their respective components, as opposed to keeping all .scss files in a single styles folder, which becomes hard to manage as the project grows.
This is a link of our repo just before conversion to styled-components
https://github.dev/Web-Dev-Path/web-dev-path/blob/e5fd0ab9b45bc32f2564c81d635d80ca4d9241c3/styles/ContactUsCards.module.scss
Were there any complications while making this change?
No
How to replicate the issue? N/A
If necessary, please describe how to test the new feature or fix.
Dark theme is populated with placeholder colors, you can test it out by adding a useEffect in any page/component to set the data-theme attribute
When should this be merged?
After review