Skip to content

Refactor CSS colors (CO-2241) #23

Merged
merged 2 commits into from
Oct 15, 2021
Merged

Conversation

arlen
Copy link
Contributor

@arlen arlen commented Oct 14, 2021

No description provided.

Copy link
Contributor

@Ioannis Ioannis left a comment

Choose a reason for hiding this comment

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

LGTM

* Colors use the convention --cmg-color-hue-number with optional comments to describe
* how they are used. "number" does not imply color value.
*/

Copy link
Contributor

@Ioannis Ioannis Oct 15, 2021

Choose a reason for hiding this comment

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

Hi @arlen i have a question.

The color file is fairly simple, and you’ll find the the numbers have no bearing on the color values

You mean that they do not imply ordering, from lighter to darker or vise versa. We should provide that information in the css file. It is not clear if a color with the postfix 001 is darker or lighter from the same one with the postfix 005.
We also have this comment:

"number" does not imply color value.

but it is not very self-explanatory, at least for me.
Perhaps we should add more text like what you provided in one of your comments:

use a numbering convention where 001 is whatever we think most important and then just add to it from there.

Bottom line, we might need one more line in the comment section that will better explain the numbering convention. Also, if we plan to have a wiki page, we should provide the link to the wiki page as well. Other than that, it looks great +1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the description in the colors file - we may revisit this, but it's slightly improved. :-)

@arlen arlen merged commit af14166 into COmanage:develop Oct 15, 2021
@arlen arlen deleted the feature-co2241 branch March 31, 2023 14:47
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants