Thanks!

Please create a 16px version as well. Monochrome icons need both sizes.

Please use a more hierarchical name, e.g. "font-enable" and "font-disable", to match the other font icons we have such as "font-size-up" and "font-face"

In D23942#531086, @broulik wrote:

Please use a more hierarchical name, e.g. "font-enable" and "font-disable", to match the other font icons we have such as "font-size-up" and "font-face"

OK. Then need to update kfontinst, too.

Oof, all of the font icons that contain an 'A' need to be cleaned up someday. The 'A's aren't even 16px tall in the 22px icons and the line thickness and alignment is all over the place. I guess that's something for another patch. For now, we should maintain consistency with the existing font icons.

@ndavis, @broulik, @ngraham, I made some change. Can you have another look?

In D23942#531279, @guoyunhe wrote:

@ndavis, @broulik, @ngraham, I made some change. Can you have another look?

I just noticed that the "No" symbol is backwards. Once you fix that and the hardcoded colors, this patch will be ready to land.

Shouldn't the checkmark be green?

In D23942#531330, @ngraham wrote:

Shouldn't the checkmark be green?

FWIW, both should be black/white. Disabling is not a destructive action like removing.

In D23942#531337, @GB_2 wrote:

In D23942#531330, @ngraham wrote:

Shouldn't the checkmark be green?

FWIW, both should be black/white. Disabling is not a destructive action like removing.

We use red for disabling as well. Perhaps we need to work on the HIG a bit. One of the problems with only using black and white is that the emblems become harder to notice.

I just followed the email icons' color: check mark is black/white, disable mark is red.

@ndavis it looks strange when I removed the inline style

I was asked to remove the hard code color like: fill="currentColor" and fill="#da4453". But I see other icons have something like style="fill:currentColor". Can anyone tell me correct way of applying colors?

In D23942#531349, @guoyunhe wrote:

I was asked to remove the hard code color like: fill="currentColor" and fill="#da4453". But I see other icons have something like style="fill:currentColor". Can anyone tell me correct way of applying colors?

Just fill="currentColor" is correct.

@GB_2 is it correct now?

In D23942#531364, @guoyunhe wrote:

@GB_2 is it correct now?

There's still a hardcoded style="fill:#da4453" in the disable icons.

Changed to currentColor

@ndavis can you give review? thanks!

I still feel like I'd prefer the checkmark to be green. I also feel like I understand the argument that red isn't the best color to use for disabling something since it isn't destructive. Maybe we should use the orange color for that?

Definitely need clarification in the HIG.