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

Migrate to SCSS #429

Merged
merged 2 commits into from
Sep 9, 2023
Merged

Migrate to SCSS #429

merged 2 commits into from
Sep 9, 2023

Conversation

smartspot2
Copy link
Member

Closes #345.

The existing CSS organization sucks: there was one huge styles.css file, containing a messy mashup of various styles for various components. We had begun using separate CSS files later on, but the weird dependencies between classes across all of these files, along with bad defaults created a lot of duplicated CSS code.

This PR aims to make the styles a lot more organized, utilizing SCSS to allow for more advanced functionality.

  • Base styles were created for buttons, forms, modals, etc., and additional global variables were created for various lengths, to allow for less repeated styling code.
  • A lot of existing styles were also replaced and revamped with more consistent styling across the entire site. This makes the UI a lot more cohesive and intuitive.
  • All of the CSS is now in separate files, grouped (approximately) by the page they affect. These styles are then imported from the respective TSX files. The original styles.css file is deleted.
  • Webpack was further configured to compile and minify (in production) these CSS files.

⚠️ This PR will break all styling from other PRs! This is simply due to the change in directory of the CSS files; most issues will be resolved by simply moving the CSS file into the correct location and importing it from the component. Some of these changes may also visually break some new components not merged into master yet, but these should generally be rare, as typically new components/features use their own styling. In the same vein, these new components should also try to use some of the shared base styles instead of creating their own styles, wherever possible.

In the future (as this PR is long enough as is), we can think about adding scoped styling, forcing styles to be imported explicitly in each component file. This would protect against any name clashes (with automatic suffixes added to each class), and also ensures that we import all of the styles we need, rather than silently stealing styles from other components.

(This is a beefy PR, but the vast majority of the changes are just copying over CSS styles to new files, and from the package-lock.json file changes.)

@smartspot2 smartspot2 added the enhancement New feature or request label Aug 12, 2023
@smartspot2 smartspot2 self-assigned this Aug 12, 2023
@cypress
Copy link

cypress bot commented Aug 12, 2023

Passing run #200 ↗︎

0 78 0 0 Flakiness 0

Details:

Migrate to SCSS
Project: csm_web Commit: 8b6e1b86f2
Status: Passed Duration: 02:40 💡
Started: Sep 9, 2023 11:00 PM Ended: Sep 9, 2023 11:03 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@smartspot2 smartspot2 force-pushed the feat/css branch 3 times, most recently from 9221f25 to d93bb10 Compare August 13, 2023 01:44
@smartspot2 smartspot2 marked this pull request as ready for review August 13, 2023 01:45
edwardneo
edwardneo previously approved these changes Aug 16, 2023
Copy link
Contributor

@edwardneo edwardneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good! I don't know too much about Cypress, so I'll let Eric take a deeper look at that.

@ericyche
Copy link
Contributor

it works on my end but i'll take another look after conflicts are resolved

Copy link
Contributor

@ericyche ericyche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it good. thank you alec

Copy link
Contributor

@edwardneo edwardneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me -- rebase and merge!

Add base styles for buttons, modals, forms, etc.
Split original styles file into multiple grouped files
Modify styles and classes to be more consistent
Set up webpack for CSS compilation
Move style files out of static folder
@smartspot2 smartspot2 merged commit ad5844a into master Sep 9, 2023
14 checks passed
@smartspot2 smartspot2 deleted the feat/css branch September 9, 2023 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor CSS
3 participants