-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
css transformed into scss #110
base: main
Are you sure you want to change the base?
Conversation
I am still not sure how many of us know how to do SCSS.. so, won't merge at the moment. I will wait for further comment & review @mkaletaa I have changes going out in a PR today too. |
Yea I totally agree with @gbowne1 not many people out here will be knowing about SCSS, so I suggest that we wait for people's review. |
SCSS is not impossible, but since the project already uses compiled SCSS coming from Bootstrap 5.2 CDN, it's a fair assumption we should target this at some point, so that the 1.0 release does not mix style types between CSS and SCSS. IMHO, SCSS can be a much cleaner option. I saw a comment in one of the files someone put there, not to make a huge styles.css. Do we need to modularize the styles into seperate stylesheets for each page? As for SCSS, It's not for new people, but it's not impossible and there are tutorials on SCSS. We would have to test compile times at the browser, and we are already at 1.0 - 1.2 seconds at least according to Google Lighthouse. Edit: SASS/SCSS tutorial: https://www.w3schools.com/sass/ |
Hey @mkaletaa ! I would like to add a little suggestion One of your changes states:
But with SCSS we can take it like:
Now what is the advantage of the above refactoring ?
We can club both the above blocks into one like:
Reference: SCSS Docs |
SASS/SCSS migration can be done at almost any point. I like your points. It may take a few of us to get it right. I just feel like we should not be mixing style types, i.e. SCSS and CSS. Because Bootstrap CDN sends out SCSS, I feel it may benefit from using a single type, not both. Using CDN's would keep the end user from having to redownload the new version of libraries, unless we provide bash shell scripts or powershell scripts to download the new versions. Using CDN's would mean There are CSS to SASS/SCSS conversion tools out there that are great too. I feel like we could all work on this change.. even if its placed in its own branch and migrated when its done and tested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conflicting changes must be resolved
I am not sure what to do about this. I looked at the files but did not see anything that I thought may be wrong. @pranavmadhu01 |
the branch has merge conflicts . It should be resolved before merging |
Am aware this needs fixing. I will look at it this weekend. @mkaletaa @pranavmadhu01 |
I changed css files into scss files and used watch sass extension which automatically created dist folder with css files. I also changed paths to css files in html files.