Removing obsolete currentColor CSS value

[webmandesign]

Fixes #12328

[webmandesign]

I've found some additional questionable usages of currentColor, possibly also need an update:

1. In packages/block-library/src/table/editor.scss file on line 30 the currentColor was recently changed to value of $black. I'm not sure if this is necessary. I suggest removing $black as in my original PR Removing redundant currentColor CSS value #12329 so we use text color instead of forcing a specific black color on borders. Can I go ahead and change this, what do you think?

2. OPTIONAL: In packages/editor/src/components/inserter-list-item/style.scss file the currentColor values could be replaced also with inherit.

3. OPTIONAL: In packages/components/src/button/style.scss file the currentColor values could be replaced also with inherit.

What do you say guys? @jasmussen @kjellr @gziolo

PS: I'm really sorry I haven't made it for 5.1 release... :(

[jasmussen]

Thanks so much for your work. Don't feel bad that this didn't make 5.1, it will land in WordPress eventually, and even before that it will land in the plugin release, so you can start using it so long as you have the plugin enabled. Your contributions are very valuable, and in its current state, it's 👍 👍 ready to go!

However since you also wrote some good thoughts around additional changes, those might be good to get in, and then get a final review.

Specificallhy, options 2 and 3 on your list seem fine to me to go ahead and fix.

For option 1, let's get some feedback from @talldan, which I believe made some changes to that file recently. Do you see any reason we can't replace the $black variable with, well, nothing?

[webmandesign]

Thanks @jasmussen Re 2 and 3, the thing is that it's just about what you guys prefer to use there. Both currentColor and inherit should return the same value and both have sufficient browser support. I personally tend to use inherit in such cases. So, if that's really fine with you too, I will update those values.

I'll wait for @talldan's feedback to proceed with removing $black.

[jasmussen]

Yep, inherit if the behavior is essentially the same, sounds good to me.

[talldan]

Hi @webmandesign - the change for the table block looks good 👍.

I don't think my change to $black should have made it in 🤦‍♂️. Originally my PR introduced the ability to change the color of the text, which is why I changed it to $black, but then that functionality was dropped from the PR. It looks as though this line wasn't reverted.

[webmandesign]

@talldan Thank you for info. I've went ahead and removed the $black from table border too.

All is fixed now.

The only instances left with currentColor in styles are fill: currentColor;, which is perfectly correct and don't require any update.

[webmandesign]

Hey guys, any update on this please?

[jasmussen]

Thanks for your work and patience. I'm going to put this in a milestone so it's guaranteed a few eyes soon. In the mean time, it needs a good rebase to solve the conflicts.

[webmandesign]

Thanks @jasmussen I'll do my best to rebase, though I don't have much luck with them ;)

[jasmussen]

If it's easier to start a new branch with the small changes discussed here, that's okay too, just ping me on the new branch and I'll milestone that — sorry about the delay.

[webmandesign]

Hi @jasmussen, I'm sorry for delay, but it seems I was able to match the commits with current master branch. Please double check, just in case, but from my point of view everything seems to be fine. Thank you!

[jasmussen]

Thank you!

[talldan]

Merging this since it has the 👍 of approval.

[webmandesign]

Thank you!