Blockbase: Add support for custom palettes customization #4098

MaggieCabrera · 2021-06-25T11:22:29Z

Changes proposed in this Pull Request:

This is a WIP. The goal is to give the user optional palettes provided by the theme's designers. Right now I'm using the code from https://github.com/HardeepAsrani/o2 as a starting point but this is a list of what I hope this will be able to do:

Get the default palette from theme.json and have it as the default option and available for users to go back to (instead of resetting each color one by one, they can reset the whole palette)
Grab the rest of the palettes from theme.json too. They should probably be under custom and we can provide labels for them there. Later, when Gutenberg supports mulitple plaettes, we can reuse those custom variables and assign them where appropiate for FSE goodness.
This PR doesn't handle the functionality that allows for the actual color updates yet.

MaggieCabrera · 2021-06-25T11:24:06Z

@kjellr if you have any input on this UI, I would appreciate your 👀

kjellr · 2021-06-25T12:06:14Z

Thanks! We talked about this on slack, but noting it here for posterity: Let's try to adjust the visual treatment of the palettes to be similar to the Featured palettes treatment on WordPress.com:

MaggieCabrera · 2021-06-25T12:32:40Z

Updated the styles of the palettes. I added a border color to mark the selected palette that the color annotations plugin does not have, since we are using radio buttons, I think it's important not to lose the checked value visual cue:

scruffian · 2021-06-25T16:09:27Z

I think this looks good to merge, but it would be good to have some more eyes on it.

pbking · 2021-06-25T16:36:01Z

blockbase/inc/wp-customize-color-palettes.php

+			'colors' => $default_palette_setting,
+		);
+
+		$custom_palettes = $theme_json['settings']['custom']['colorPalettes'];


I'm getting an error here w/ Quadrat due to there being no palette's currently defined in theme.json
Likely the Quadrat palette options weren't included in the change (yet).
But seems reasonable that a BB child might not ship with any customizations so probably a relevant use-case.

Notice: Undefined index: colorPalettes in /var/www/html/wp-content/themes/themes/blockbase/inc/wp-customize-color-palettes.php on line 33 Warning: Invalid argument supplied for foreach() in /var/www/html/wp-content/themes/themes/blockbase/inc/wp-customize-color-palettes.php on line 34

yeah, that fixed disappeared on a conflict, let me get it back

should be fine now

Pushed this small change to only render these controls if there are alternatives to default to show. Also addressed that error that seemed to stick around despite the recent change.

blockbase/inc/wp-customize-color-palettes.php

pbking · 2021-06-25T17:04:54Z

I'm of the opinion that the alternative palettes should be BUTTONS instead of RADIO BUTTONS. The action feels more like "apply these colors to my color pickers" rather than "select these colors to use" and BUTTON rather than RADIO BUTTON seems to reflect that better.

Otherwise we will invite the situation where an option might be chosen and then customized. In this situation the RADIO BUTTON would need to be unselected (no option selected) otherwise that option couldn't be returned to without selecting something else and returning. If we do keep it as a RADIO BUTTON then I suggest another stylistic mechanism to signify that that option is selected (It's difficult to see.)

That's just an opinionated nit-pick. Otherwise this looks FANTASTIC and I would be happy to ship as-is and iterate.

jffng

Tested in all four themes and it is a thing of beauty 👏

I did run into an issue with Quadrat. Because of the way our merge / build step works for child theme, the colorPalettes from blockbase are added, which results in the following error after I build Quadrat's theme.json:

If I provide a new palette with only the three colors per palette using the same palette slug (e.g. featured-1) and run build, I still get the above error because the merge appends the values to the colors. In this instance, I think what needs to happen is that the entire key colorPalettes object is replaced with the child-theme.json value, rather than the recursive merge.

jffng · 2021-06-25T17:26:01Z

blockbase/theme.json

@@ -116,6 +116,38 @@
 				"secondary": "var(--wp--preset--color--secondary)",
 				"selection": "var(--wp--preset--color--selection)"
 			},
+			"colorPalettes": {
+				"palette-1": {
+					"label": "Featured",


I think there is a mechanism now for labels for presets to get translated, but is there a way yet to mark that a custom variable needs to be translated?

I think we do it like this. Since we haven't done this for any of the strings that we have (such as the template parts) I think this merits its own separate PR

scruffian · 2021-06-25T20:01:30Z

I did run into an issue with Quadrat. Because of the way our merge / build step works for child theme, the colorPalettes from blockbase are added, which results in the following error after I build Quadrat's theme.json:

I wonder if this means we should switch to using an array...

MaggieCabrera · 2021-06-28T07:04:30Z

I wonder if this means we should switch to using an array...

I think we kind of have to, it's what's consistent with what we've done for other variables like this

blockbase/inc/wp-customize-palette-control.php

MaggieCabrera

I tested Blockbase and selected one of the palettes, then I changed to Quadrat and the palette that was selected for Blockbase was showing up for the customizer too (while its colors showed up something else)

…o default.

…o default. (fix)

scruffian · 2021-06-28T12:48:11Z

I tested Blockbase and selected one of the palettes, then I changed to Quadrat and the palette that was selected for Blockbase was showing up for the customizer too (while its colors showed up something else)

I think this is because the Global Styles settings weren't populating back into the customizer. I had to put back the settings file I removed because I couldn't find a way to set a dirty value.

MaggieCabrera added the [Theme] Blockbase label Jun 25, 2021

MaggieCabrera requested review from pbking and jffng June 25, 2021 16:21

MaggieCabrera self-assigned this Jun 25, 2021

MaggieCabrera requested a review from a team June 25, 2021 16:22

MaggieCabrera marked this pull request as ready for review June 25, 2021 16:22

pbking reviewed Jun 25, 2021

View reviewed changes

scruffian reviewed Jun 25, 2021

View reviewed changes

blockbase/inc/wp-customize-color-palettes.php Outdated Show resolved Hide resolved

jffng reviewed Jun 25, 2021

View reviewed changes

MaggieCabrera commented Jun 28, 2021

View reviewed changes

blockbase/inc/wp-customize-palette-control.php Outdated Show resolved Hide resolved

MaggieCabrera commented Jun 28, 2021

View reviewed changes

MaggieCabrera and others added 12 commits June 28, 2021 11:00

first pass at color palette controls without functionality

933aa2c

made palettes look like color annotations plugin

284b68e

make color selection functional

2f66ce8

get palettes from theme.json

47c697c

rename file

50ac20e

rename colors preview file

dd15473

rename files

d55abbd

more file renaming for clarity

753e1ef

simplify settings

ec413a7

rebuilt mayland and seedlet with alternative palettes

049c68f

renamed default template, dynamic theme name

8bf752c

remove space

e73bf28

MaggieCabrera and others added 9 commits June 28, 2021 11:02

Added default Varia/Seedlet palettes to blockbase

ebf38dc

check if the custom palette array is empty

a61c6ac

Only render color palette picker controls if there are alternatives t…

e0e9522

…o default.

Only render color palette picker controls if there are alternatives t…

5808c73

…o default. (fix)

Removed commented code

555c9b1

change palettes to an array

ddc7b0b

chagne props comment

df3dcb0

removed unnecesary credit

842dc5d

undo last commit, brought back the credits

28f033b

scruffian force-pushed the try/blockbase-customizer-alternative-palettes branch from c135fe6 to 28f033b Compare June 28, 2021 10:03

readd the settings file

2a80ac9

rebuild children

5d328b8

scruffian approved these changes Jun 28, 2021

View reviewed changes

scruffian merged commit 9e49d27 into trunk Jun 28, 2021

scruffian deleted the try/blockbase-customizer-alternative-palettes branch June 28, 2021 13:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blockbase: Add support for custom palettes customization #4098

Blockbase: Add support for custom palettes customization #4098

MaggieCabrera commented Jun 25, 2021 •

edited by scruffian

Loading

MaggieCabrera commented Jun 25, 2021

kjellr commented Jun 25, 2021 •

edited

Loading

MaggieCabrera commented Jun 25, 2021

scruffian commented Jun 25, 2021

pbking Jun 25, 2021 •

edited

Loading

MaggieCabrera Jun 25, 2021

MaggieCabrera Jun 25, 2021

pbking Jun 25, 2021

pbking commented Jun 25, 2021

jffng left a comment

jffng Jun 25, 2021

MaggieCabrera Jun 28, 2021

scruffian commented Jun 25, 2021

MaggieCabrera commented Jun 28, 2021

MaggieCabrera left a comment

scruffian commented Jun 28, 2021

Blockbase: Add support for custom palettes customization #4098

Blockbase: Add support for custom palettes customization #4098

Conversation

MaggieCabrera commented Jun 25, 2021 • edited by scruffian Loading

Changes proposed in this Pull Request:

MaggieCabrera commented Jun 25, 2021

kjellr commented Jun 25, 2021 • edited Loading

MaggieCabrera commented Jun 25, 2021

scruffian commented Jun 25, 2021

pbking Jun 25, 2021 • edited Loading

Choose a reason for hiding this comment

MaggieCabrera Jun 25, 2021

Choose a reason for hiding this comment

MaggieCabrera Jun 25, 2021

Choose a reason for hiding this comment

pbking Jun 25, 2021

Choose a reason for hiding this comment

pbking commented Jun 25, 2021

jffng left a comment

Choose a reason for hiding this comment

jffng Jun 25, 2021

Choose a reason for hiding this comment

MaggieCabrera Jun 28, 2021

Choose a reason for hiding this comment

scruffian commented Jun 25, 2021

MaggieCabrera commented Jun 28, 2021

MaggieCabrera left a comment

Choose a reason for hiding this comment

scruffian commented Jun 28, 2021

MaggieCabrera commented Jun 25, 2021 •

edited by scruffian

Loading

kjellr commented Jun 25, 2021 •

edited

Loading

pbking Jun 25, 2021 •

edited

Loading