gbowne1 / spmssite

The code for my old business website.
GNU General Public License v3.0
8 stars 31 forks source link

css transformed into scss #110

Open mkaletaa opened 2 years ago

mkaletaa commented 2 years ago

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.

gbowne1 commented 2 years ago

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.

dk3775 commented 2 years ago

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.

gbowne1 commented 2 years ago

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/

Pathholder1806 commented 2 years ago

Hey @mkaletaa ! It is really good to have you contribute to this and kudos to you for having the changes done.

I would like to add a little suggestion As per my existing knowledge of SCSS & SAAS. There is a scope of more refactoring in the SCSS files. For eg

One of your changes states:

.mode__toggle:hover {
  transition: .1s;
  width: 42px;
  height: 42px;
}

But with SCSS we can take it like:

.mode__toggle {
  &:hover {
    transition: .1s;
    width: 42px;
    height: 42px;  
  }
}

Now what is the advantage of the above refactoring ? For eg we have some code like

.enlarge {
  font-size: 14px;
  transition-property: font-size;
  transition-duration: 4s;
  transition-delay: 2s;
}
.enlarge:hover {
  font-size: 36px;
}

We can club both the above blocks into one like:

.enlarge {
  font-size: 14px;
  transition: {
    property: font-size;
    duration: 4s;
    delay: 2s;
  }

  &:hover { font-size: 36px; }
}

Reference: SCSS Docs And likewise there are other instances also of parent-child relationship in the old CSS file which can be refactored using SCSS So I guess at this stage still we are not fully exploiting the advantages of SCSS for a cleaner code. I am also not a master with it so please feel free to add your review to it also.

gbowne1 commented 2 years ago

@Pathholder1806

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.

gbowne1 commented 1 year ago

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

pranavmadhu01 commented 1 year ago

the branch has merge conflicts . It should be resolved before merging

gbowne1 commented 1 year ago

Am aware this needs fixing. I will look at it this weekend.

@mkaletaa @pranavmadhu01