Skatepark: dynamic duotone support #4740
Conversation
MaggieCabrera
added
the
[Theme] Skatepark
Should we...
or
I think option 1 makes sense, but I thought it was worth discussing :) |
Option 1 makes more sense to me because the number of blocks where the filter may be used can vary in the future and we'd have to keep maintaining it or look for every instance of the filter all over the json. If we change the default filter though we would be losing the green option on the filter menu for the users to chose and duplicating one of the other options. Maybe we should define the green-black filter and the default one, so we don't lose the green one but we'd still always have a duplicate one. Now I'm not so sure! |
|
We should consider if we want to add an option to disable duotone from the customizer as a default option for all the blocks. |
scruffian
force-pushed
the
skatepark-duotone-dynamic
branch
from
8d14624 to
36ca3a8
Compare
|
I added a commit to add a updated default filter to the user CPT for Global Styles. This revealed a bug in the way Duotone works: WordPress/gutenberg#35650 I think we're going to get blocked by this :/ |
We could try option 2, instead of changing the default filter, we just assign a different one to the blocks that need it. It will also help with adding an option to turn duotone off altogether. |
|
I've been thinking about this and option 2 is not a good idea because it wouldn't work with custom colors anyway, just with the preset palettes from the theme :( |
What about an option 3 where we create a new filter? |
We should pair on this, I think! |
|
Sorry in advance because this code is probably horrible, but it actually works in the frontend! 🎉
What's left to do:
Thanks @mikachan for your help with this! |
|
Also, I don't think this PR works for testing the GB PR anymore, sorry! |
MaggieCabrera
force-pushed
the
skatepark-duotone-dynamic
branch
from
718d59d to
d60b60c
Compare
| const colorValues = getValuesFromColors( colors ); | ||
|
|
||
| if ( document.querySelector( '#wp-duotone-default-filter' ) ) { | ||
| updateDuotoneFilter( 'wp-duotone-default-filter', colorValues ); |
There was a problem hiding this comment.
It's a little tricky to understand why there is no hash on this string but there is on line above. It might be better to just move the hash here for consistency.
| if ( userColorDuotone ) { | ||
| let colors = palette.map( ( paletteItem ) => paletteItem.color ); | ||
| //we are inverting the order when we have a darker background so that duotone looks good. | ||
| colors = colors.sort( ( first, second ) => { |
There was a problem hiding this comment.
We can write this in one line
| } ); | ||
| const colorValues = getValuesFromColors( colors ); | ||
|
|
||
| if ( document.querySelector( '#wp-duotone-default-filter' ) ) { |
There was a problem hiding this comment.
We could move this check inside the function
|
This is looking good, but I don't see all the filters in the editor: |
I tried to merge the user's custom filter with theme preset ones and it's mostly working but in some cases some of the colors are not saving correctly if you var_dump the result of the array_merge from my commit. I can't seem to find what's causing that! |
MaggieCabrera
force-pushed
the
skatepark-duotone-dynamic
branch
from
03bca65 to
e480a81
Compare
Co-authored-by: Sarah Norris <sarah@sekai.co.uk>
scruffian
force-pushed
the
skatepark-duotone-dynamic
branch
from
e03f547 to
c78c7e2
Compare
This reverts commit f1a3133.
This reverts commit f1a3133.
* Blockbase: Use the Global Styles rest API in the customizer * Color and Fonts customizer: changing implementation * Color and Fonts customizer: changing implementation * unlinting * unlinting * Customizer: handling when settings or styles are not arrays * Revert "Skatepark: dynamic duotone support (#4740)" This reverts commit f1a3133. * Blockbase: Use the Global Styles rest API in the customizer * Color and Fonts customizer: changing implementation * unlinting * unlinting * Customizer: handling when settings or styles are not arrays * Revert "Skatepark: dynamic duotone support (#4740)" This reverts commit f1a3133. * remove unused function * move settings to the theme key * revert skatepark changes * revert skatepark changes * Renaming variables * Reverting dded23b * re-adding delete_transient * Remonving condition to unset fonts when default is selected Co-authored-by: Ben Dwyer <ben@scruffian.com>
Changes proposed in this Pull Request:
This PR applies the changes from WordPress/gutenberg#34667 to Skatepark. I have removed the duotone markup from block patterns and templates and defined it directly on theme.json for the affected blocks.
To test this, check that all the images in Skatepark show the duotone filter. Try inserting new ones too :D I went ahead and removed some markup issues from block patterns, check that none of the blocks throw any errors too.
TODO: we need to implement the change to the default filter when the user selects a new color palette on the customizer.
Closes #4411 #4329