New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use color styles on the editor even if the classes were not set #7539

Merged
merged 1 commit into from Jul 4, 2018

Conversation

Projects
None yet
2 participants
@jorgefilipecosta
Member

jorgefilipecosta commented Jun 25, 2018

Description

Fixes: #6585
Before the editor just used the same classes on the frontend to apply color styles. If the classes were not set the editor would not apply them.

While for frontend view the current behaviour is the expected, on the editor is the themes passed us the color value we should use it even if the class was not created.

How has this been tested?

Add a palette that sets a color without implementing the class, verify the color was used in the editor.
e.g:

		add_theme_support( 'editor-color-palette',
			array(
				'name' => 'test red',
				'color' => '#f00',
			)
		);

@jorgefilipecosta jorgefilipecosta self-assigned this Jun 25, 2018

@jorgefilipecosta jorgefilipecosta requested a review from WordPress/gutenberg-core Jun 28, 2018

@jorgefilipecosta jorgefilipecosta added this to the 3.2 milestone Jun 28, 2018

@gziolo

This comment has been minimized.

Member

gziolo commented Jun 29, 2018

Code wise looks good. Travis in unhappy though.

Can you share the link to the previous setup for reference, the one which uses a class name?

@jorgefilipecosta

This comment has been minimized.

Member

jorgefilipecosta commented Jun 29, 2018

Hi @gziolo thank you for the review!

Code wise looks good. Travis in unhappy though.

I rebased, and the tests are now passing, I'm not sure why Travis was unhappy.

Can you share the link to the previous setup for reference, the one which uses a class name?

I'm not sure what you mean by "previous setup" I'm probably missing something. The only change we are doing in this PR is that before if we had a class for color, on the editor we just used the class without setting an inline style for the color. Right now, even if a color class exists we set an inline style anyway, so we are not dependent on the theme providing a class to show the colors on the editor correctly.
Before we did:
backgroundColor: backgroundColor.class ? undefined : backgroundColor.value
(we added the inline style only if the class was not set)
Now we do:
backgroundColor: backgroundColor.value,
(always add the inline style)

@gziolo

gziolo approved these changes Jul 4, 2018

Okey, let's get it in 👍

@jorgefilipecosta jorgefilipecosta merged commit b59363f into master Jul 4, 2018

2 checks passed

codecov/project 47.09% remains the same compared to 124647e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jorgefilipecosta jorgefilipecosta deleted the update/use-styles-on-editor-even-if-class-wasnot-set branch Jul 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment