Make color editable

[nielslange]

Fixes #6

Note

In i1 of this block, the merchant must be able to select the color of the message and the progress bar. This PR aims to add this functionality.

Screenshots

Named colors

Editor:

Frontend:

Custom colors

Editor:

Frontend:

Test instructions

• Check out this PR.
• Create a test page.
• Add the block Free Shipping Progress Bar to the test page.
• Click on the block to open the Inspector Controls.
• Adjust the message color and the progress bar color using named colors.
• Ensure that the color change in the editor.
• Save the test page.
• Ensure that the color change in the frontend.
• Adjust the message color and the progress bar color using custom colors.
• Ensure that the color change in the editor.
• Save the test page.
• Ensure that the color change in the frontend.

[nielslange]

@senadir: As requested, I've implemented the colors using withColors and getColorClassName. Implementing the font color and the background color was no problem. However, to implement the border color, I had to copy the following CSS definitions from Gutenberg into this PR:

.has-pale-pink-border-color {
	border-color: #f78da7;
}
.has-vivid-red-border-color {
	border-color: #cf2e2e;
}
.has-luminous-vivid-orange-border-color {
	border-color: #ff6900;
}
.has-luminous-vivid-amber-border-color {
	border-color: #fcb900;
}
.has-light-green-cyan-border-color {
	border-color: #7bdcb5;
}
.has-vivid-green-cyan-border-color {
	border-color: #00d084;
}
.has-pale-cyan-blue-border-color {
	border-color: #8ed1fc;
}
.has-vivid-cyan-blue-border-color {
	border-color: #0693e3;
}
.has-vivid-purple-border-color {
	border-color: #9b51e0;
}
.has-white-border-color {
	border-color: #fff;
}
.has-very-light-gray-border-color {
	border-color: #eee;
}
.has-cyan-bluish-gray-border-color {
	border-color: #abb8c3;
}
.has-very-dark-gray-border-color {
	border-color: #313131;
}
.has-black-border-color {
	border-color: #000;
}

I saw that you created WordPress/gutenberg#16848, but this hasn't been addressed yet. I wonder if my approach is the correct way or if you can think of a better way.

[senadir]

I took a quick glance, I will be waiting for when you update this to include currenColor

[senadir]

Hey Niels, this is not working so far. You're on the correct track, but not there yet.

Please make sure to test your PR before making it available for a review. This might have been working on a previous iteration but isn't working right now and should have been fully retested.

If it's actually working for you fine, then we can try to understand why my version isn't working yet.

[nielslange]

Please make sure to test your PR before making it available for a review. This might have been working on a previous iteration but isn't working right now and should have been fully retested.

It's strange that the PR was is working on your end, as it works as expected on my end. Before pushing a change, I always check if the change is working as expected. Of course, I might oversee one or the other issue now and then. But in general, I do not push untested code and requesting a review.

If it's actually working for you fine, then we can try to understand why my version isn't working yet.

It would be great if we could look at my code together, to see why it's working on my end, to prevent that this inconsistency is happening again in the future.

[senadir]

It would be great if we could look at my code together, to see why it's working on my end, to prevent that this inconsistency is happening again in the future.

This was a dependency issue, as I was running Gutenberg plugin. We support Gutenberg in our code so we should try to fix this if it's possible.

[senadir]

Also, custom colors are not working, Gutenberg active or not, this is because they're not being used in your code, and you're only using the color tokens.

[nielslange]

Also, custom colors are not working, Gutenberg active or not, this is because they're not being used in your code, and you're only using the color tokens.

I replaced the current color settings with Gutenberg's native color support. Both color variants and custom colors are now working both in the editor and on the frontend independent of the activated theme.

You can go ahead and do another review, once you have time doing so. 😀

[senadir]

While both custom and predefined colors are working fine. I'm seeing a single option now. Is this the expected behavior now? if so, can you update the PR description and double check in the project thread?

[nielslange]

While both custom and predefined colors are working fine. I'm seeing a single option now. Is this the expected behavior now? if so, can you update the PR description and double check in the project thread?

I've updated the screenshots in the PR description accordingly and in the next weekly update in the project thread, I'll point out why I moved to the native Gutenberg color settings and why I'm only using the text color settings and not the background color settings.

[senadir]

I see you removed the background color option. I think we can use a mix of withColor and supports.color to achieve both goals.
It's true that supports.color is the recommended way to add colors, but it doesn't fit all use cases, and in that case, we can also withColor

[nielslange]

@senadir I adjusted the color settings using the native Gutenberg color support for both the text and the background color. Initially, I faced the problem that the custom colors were only visible in the editor. I overcame this with the following section:

woocommerce-free-shipping-progress-bar-block/woocommerce-free-shipping-progress-bar-block.php

Lines 47 to 55 in fc403cd

	// Add the custom background color to the attributes array.
	if ( ! $attributes['backgroundColor'] && $attributes['style']['color']['background'] ) {
	$attributes['backgroundColor'] = $attributes['style']['color']['background'];
	}
	// Add the custom text color to the attributes array.
	if ( ! $attributes['textColor'] && $attributes['style']['color']['text'] ) {
	$attributes['textColor'] = $attributes['style']['color']['text'];
	}

This section adds the custom colors to the attributes so that they can be added to the block as data-attributes. Before this change, the outer element of my block looked like this:

<div data-style="{"color":{"background":"#bf616a","text":"#5e81ac"}}" data-current-total="15" data-free-shipping-from="50" class="wp-block-nielslange-free-shipping-progress-bar">....</div>

Due to my change, the outer element now looks like this:

<div data-background-color="#bf616a" data-text-color="#5e81ac" data-current-total="15" data-free-shipping-from="50" class="wp-block-nielslange-free-shipping-progress-bar">...</div>

In addition, I noticed that not all themes were showing the named colors in the editor, for example, the Twenty Nineteen theme. I overcame this problem with the following section:

woocommerce-free-shipping-progress-bar-block/src/block.js

Lines 22 to 43 in fc403cd

	// Prepare message color.
	const messageColor =
	textColor ?? colorProps?.style?.color ?? 'currentColor';
	const messageClass = getColorClassName( 'color', messageColor );
	const messageStyle = { color: colorProps?.style?.color ?? messageColor };
	// Prepare border class and style.
	const borderColor =
	backgroundColor ?? colorProps?.style?.backgroundColor ?? 'currentColor';
	const borderClass = getColorClassName( 'color', borderColor );
	const borderStyle = {
	color: colorProps?.style?.backgroundColor ?? borderColor,
	};
	// Prepare progress class and style.
	const progressColor =
	backgroundColor ?? colorProps?.style?.backgroundColor ?? 'currentColor';
	const progressClass = getColorClassName( 'color', progressColor );
	const progressStyle = {
	width: progressWidth,
	color: colorProps?.style?.backgroundColor ?? progressColor,
	};

I'm excited to hear your thoughts on these changes.

[mikejolley]

Left some comments. The biggest issue are the PHP notices on first running this PR. They need to be fixed with isset.

R/e the functionality, I think the main thing lacking (from experience using similar customisation controls on other platforms) is control of both the progress indicator, and the track. So if possible, can you add separate controls for those colours?

As well as progress and track, the border is always grey. What about generating a border colour from the track colour? Or just using track for border too.

Finally, and not related entirely to this PR, have you considered using the HTML5 <progress> element instead of a div?

[nielslange]

Left some comments. The biggest issue are the PHP notices on first running this PR. They need to be fixed with isset.

Great catch and I've already adjusted that.

R/e the functionality, I think the main thing lacking (from experience using similar customisation controls on other platforms) is control of both the progress indicator, and the track. So if possible, can you add separate controls for those colours?

As well as progress and track, the border is always grey. What about generating a border colour from the track colour? Or just using track for border too.

Funny enough, in an earlier version, the border color was derived from the process bar background color. I then moved away from that solution towards using the same color as for the quantity buttons. That said, I moved back to the initial version of using the same color both for the process bar and the border.

Finally, and not related entirely to this PR, have you considered using the HTML5 <progress> element instead of a div?

I have, and I voted against that for design reasons. The HTML5 <progress> element offers very limited styling options and even these already limited options are not supported by every browser. To give merchants the highest level of flexibility, I decided to implement the progress bar using an outer and an inner div. That way, it's possible to style the element cross-browser compatible.

Since I applied all changes, would you mind doing another review, @mikejolley?

[mikejolley]

The HTML5 element offers very limited styling options and even these already limited options are not supported by every browser.

Is this still true? I've used them before and I know css offers a reset so you essentially get 2 bars (track and progress): https://css-tricks.com/html5-progress-element/ I'd argue we should use semantic elements whenever possible even if it compromises slightly on appearance, because div soup has no meaning.

[nielslange]

Is this still true? I've used them before and I know css offers a reset so you essentially get 2 bars (track and progress): https://css-tricks.com/html5-progress-element/ I'd argue we should use semantic elements whenever possible even if it compromises slightly on appearance, because div soup has no meaning.

I agree that we should prefer semantic elements. I might have faced the problem before, since I haven't reset the default behavior. I'll give it another try.

[nielslange]

I switched to the native HTML5 progress element, and resetting the styles indeed solved the cross-browser problem I faced before. I checked the element in Chrome, Firefox, Opera and Safari and the styling looks identical in all browsers.

[mikejolley]

@nielslange bar looks good. I notice the currency symbol is missing in the message. Is this is separate issue or new?

[nielslange]

@nielslange bar looks good. I notice the currency symbol is missing in the message. Is this is separate issue or new?

The currency symbol will be implemented with #29. Can I go ahead and merge this PR or did you notice anything else that should be adjusted?