Add text color support to Columns block

[AnthonyLedesma]

Description

With the merge of #17813 columns block now supports Background color controls. This PR will add text color support to the core Columns block and is related to #16660. This uses the same method employed by the core Group block.

How has this been tested?

Tested in supported browsers and ensured color classes and custom colors are applied to the columns block within the editor and the front-end.

Screenshots

Types of changes

New feature: A new Text Color control for the Columns block, allowing folks to add a text color to their Column blocks.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR. .

[paaljoachim]

Thanks for working on this PR Anthony!

[AnthonyLedesma]

I have this tested and working locally. I will get a look at the Travis tests here soon. Sorry I didn't realize before I went out of the draft.

[paaljoachim]

It sounds like the PR is almost ready for a code review.
It will fit perfectly in with the Column block Background Color.

[AnthonyLedesma]

Hi @paaljoachim,

I would love to see this included in the columns block. I got a look at Travis and I am not sure exactly the cause of the error. Would you mind getting a look for me and possibly providing more content?

Here is the error from Travis.

There are local uncommitted changes after one or both of 'npm install' or 'npm run docs:build'!
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! gutenberg@7.4.0 check-local-changes: `( git diff -U0 | xargs -0 node bin/process-git-diff ) || ( echo "There are local uncommitted changes after one or both of 'npm install' or 'npm run docs:build'!" && exit 1 );`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the gutenberg@7.4.0 check-local-changes script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2020-02-06T20_09_25_305Z-debug.log
The command "npm run check-local-changes" exited with 1.

[paaljoachim]

Hey Anthony.

I asked if @epiqueras could take a look.

[epiqueras]

You have uncommitted lock-file changes and/or documentation changes (npm run docs:build).

[epiqueras]

None of your changes should be changing the lock-file. Try removing the lock-file changes from the PR.

[AnthonyLedesma]

@epiqueras Thank you. Was getting some strange behavior. That is all sorted now. :)

[paaljoachim]

Regarding Design: It looks like Text color setting uses the default standard also used in other blocks. For instance the setting is the same as used in the Group Block. (Followup comment made by @richtabor in the design channel on Slack.)

What is left to do in this PR? Is it ready for a code review?

[AnthonyLedesma]

What is left to do in this PR? Is it ready for a code review?

Ready for a code review from my end. 👍

[getdave]

This is looking good to me.

[jorgefilipecosta]

LGTM 👍 Thank you for submitting this PR!

As a follow-up, I think we should add some fixtures to try this attribute during the tests.

I'm also noticing a bug with contrast checking where the warning may appear multiple times:

cc: @epiqueras

cc: @mapk to be aware of this addition and have a look on the design side.

[paaljoachim]

It is great to get this merged!
Thank you Jorge!

[epiqueras]

I'm also noticing a bug with contrast checking where the warning may appear multiple times:

There is one for each managed color. This needs to use the array form of contrast checkers:

- contrastCheckers: { backgroundColor: true, textColor: true },
+ contrastCheckers: [ { backgroundColor: true, textColor: true } ],

[karmatosed]

As far as design goes, that message really needs to only show once for the color checking, otherwise, it's really a lot visually.

From an iteration view, I want to move away from the double sidebar, we should explore like other blocks bringing this into the toolbar for color selection. I am almost thinking we could hold this off and bring in over rush it in. That said, I wouldn't stand in way of it being merged as-is. @mapk cc'ing you in for final decision there. I do feel we should move away from this pattern and into how cover and navigation are going for color, with it 'by the block'.

[epiqueras]

This is already merged, but I agree, we should move towards having these in the toolbar.

[karmatosed]

Oh my goodness, I didn't refresh the page so didn't see merged! Thanks for that. Let's iterate then in next versions.

[jorgefilipecosta]

Oh my goodness, I didn't refresh the page so didn't see merged! Thanks for that. Let's iterate then.

@karmatosed, I'm sorry for merging the PR without a proper design check. As the deadline for features was approaching I needed to merge this PR. It follows exactly what the group block did and that one was already merged, so I preferred to make sure both blocks are consistent.
I guess the visual bugs common for both blocks can be addressed during the beta period, while adding the feature involving attributes etc would not be possible during the beta period.

[jorgefilipecosta]

A fix for the repetition of messages in group and columns is available at #20143. Thank you for providing the fix @epiqueras!

[ZebulanStanphill]

@karmatosed

From an iteration view, I want to move away from the double sidebar, we should explore like other blocks bringing this into the toolbar for color selection.

See #20070.