search

spree-contrib / spree_fancy

SpreeFancy is a responsive theme for Spree Commerce.
http://guides.spreecommerce.org
BSD 3-Clause "New" or "Revised" License
86 stars 187 forks source link

Scss variable naming conventions #105

Open JEphron opened 10 years ago

JEphron commented 10 years ago

I guess this is a minor cosmetic issue, but I thought it'd be worth bringing up nonetheless. I noticed that the scss variables are named by color. For example 

$c_red:         #e45353

I've overridden them in my project, and now I've got a variable named $c_red that's actually blue. Would it not be better to name them by purpose?

radar commented 10 years ago

The comment immediately after that variable definition explains the purpose:

$c_red:         #e45353 !default;  /* Error red      */

As far as I can see, this colour is used for any error dialogs... and imo red is the perfect colour to use for errors, since everywhere else uses it.

Why do you have a red that is blue?

JEphron commented 10 years ago

If the sole purpose of that variable is to determine the color of error dialogs, then perhaps it should be named $c_error

There are also $c_blue and $c_orange variables which don't seem to have any specific meaning attached to them.

If this theme is to be easily customizable then the variable names should be flexible enough to avoid confusion.

radar commented 10 years ago

@JEphron Could you please submit a PR to give the variables better names?