search

alshedivat / al-folio

A beautiful, simple, clean, and responsive Jekyll theme for academics
https://alshedivat.github.io/al-folio/
MIT License
11.34k stars 11.28k forks source link

fix: light-name-color -> color-name-light; added light variants if mi… #2750

Open CheariX opened 1 month ago

CheariX commented 1 month ago

I found that some color names used a different naming scheme than other.

E.g. $light-cyan-color should be $cyan-color-light according to the other light and dark variants. Also, I added light variants in case they were missing.

george-gca commented 1 month ago

I don't think they are exactly light variants, some of them might be just a light cyan instead of a cyan for light mode, for example.

Also if you changed the color name check if it isn't used anywhere. If it is, you should also change it there. If it isn't, why does it even exist? So maybe we should delete it.

There will be one day that one shall look at all our sass code and clean/organize it.

CheariX commented 1 month ago

I don't think they are exactly light variants, some of them might be just a light cyan instead of a cyan for light mode, for example.

Oh, this is good to know. I did not know that the light variants are supposed to be cyan for light mode instead of light cyan. Thank you.

So this particular light-cyan-color is correctly named and used in a dark theme? Then we would need a light-cyan-color-light for the light theme? Also, there are color names like green-color-bright, which sound to me like what this cyan should have been defined.

These namings are really confusing. 😃

Also if you changed the color name check if it isn't used anywhere. If it is, you should also change it there. If it isn't, why does it even exist? So maybe we should delete it.

I checked it before creating this PR but I could not find any references.

There will be one day that one shall look at all our sass code and clean/organize it.

😀

TBH, I don't care too much about this particular PR. It was just something that I stumbled upon during my last master merge. If you don't like it, feel free to close/reject it.

george-gca commented 1 month ago

Are these new colors used somewhere? If not, I don't think we should create them, even though they will be discarded by PurgeCSS.