Skip to content
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.

Sign up for GitHub

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

Fix duotone rendering in site editor #37727

Merged
merged 23 commits into from
Apr 18, 2022
Merged

Fix duotone rendering in site editor #37727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2012290
Generate duotone filter styles for useGlobalStyles
Jan 4, 2022
01de5c3
Generate duotone filter presets for useGlobalStyles
Jan 4, 2022
bfb1e11
Separate out duotone filter and stylesheet
Jan 4, 2022
b692976
Fix duotone preset metadata classes
Jan 12, 2022
c6d4113
Render duotone filters with editor.PresetSvgFilter hook
Jan 12, 2022
3e07d40
Rename Filter to PresetSvgFilter in HOC
Jan 12, 2022
3a576b1
Add path check for duotone preset hook
Jan 12, 2022
c683e0d
Use PRESET_METADATA for applying the duotone preset filter
Jan 12, 2022
6df1f72
Destructure withDuotoneFilter props
Jan 12, 2022
e3679e1
Filter on svgFilter before mapping
Jan 12, 2022
914f961
Remove unused import
Jan 12, 2022
8dc561e
Formatting for getPresetsSvgFilters
Jan 12, 2022
3dda274
Try rendering SVGs from server
Jan 25, 2022
0ca4ed7
Revert "Try rendering SVGs from server"
Feb 15, 2022
498fbe7
Prevent filters from generating top-level styles
Apr 14, 2022
7a03f72
remove the filter and import the component directly
scruffian Apr 8, 2022
6b568ed
Refactor to simplify and rename exported duotone filter
Apr 15, 2022
91a77e7
Add comment about why filters are rendered where they are
Apr 18, 2022
fc571fa
Remove svgFilter metadata
Apr 18, 2022
8e99d6f
Add comment about filtering on duotone
Apr 18, 2022
57a8d43
Revert "Prevent filters from generating top-level styles"
oandregal Apr 18, 2022
ae3e332
Pull duotone selector from block metadata if it exists
oandregal Apr 18, 2022
d75b3a5
Render duotone filter styles
oandregal Apr 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 98 additions & 62 deletions packages/block-editor/src/hooks/duotone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,82 +60,109 @@ export function getValuesFromColors( colors = [] ) {
*/

/**
* SVG and stylesheet needed for rendering the duotone filter.
* Stylesheet for rendering the duotone filter.
*
* @param {Object} props Duotone props.
* @param {string} props.selector Selector to apply the filter to.
* @param {string} props.id Unique id for this duotone filter.
* @param {Values} props.values R, G, B, and A values to filter with.
*
* @return {WPElement} Duotone element.
*/
function DuotoneFilter( { selector, id, values } ) {
const stylesheet = `
function DuotoneStylesheet( { selector, id } ) {
const css = `
${ selector } {
filter: url( #${ id } );
}
`;
return <style>{ css }</style>;
}

/**
* SVG for rendering the duotone filter.
*
* @param {Object} props Duotone props.
* @param {string} props.id Unique id for this duotone filter.
* @param {Values} props.values R, G, B, and A values to filter with.
*
* @return {WPElement} Duotone element.
*/
function DuotoneFilter( { id, values } ) {
return (
<>
<SVG
xmlnsXlink="http://www.w3.org/1999/xlink"
viewBox="0 0 0 0"
width="0"
height="0"
focusable="false"
role="none"
style={ {
visibility: 'hidden',
position: 'absolute',
left: '-9999px',
overflow: 'hidden',
} }
>
<defs>
<filter id={ id }>
<feColorMatrix
// Use sRGB instead of linearRGB so transparency looks correct.
colorInterpolationFilters="sRGB"
type="matrix"
// Use perceptual brightness to convert to grayscale.
values="
.299 .587 .114 0 0
.299 .587 .114 0 0
.299 .587 .114 0 0
.299 .587 .114 0 0
"
<SVG
xmlnsXlink="http://www.w3.org/1999/xlink"
viewBox="0 0 0 0"
width="0"
height="0"
focusable="false"
role="none"
style={ {
visibility: 'hidden',
position: 'absolute',
left: '-9999px',
overflow: 'hidden',
} }
>
<defs>
<filter id={ id }>
<feColorMatrix
// Use sRGB instead of linearRGB so transparency looks correct.
colorInterpolationFilters="sRGB"
type="matrix"
// Use perceptual brightness to convert to grayscale.
values="
.299 .587 .114 0 0
.299 .587 .114 0 0
.299 .587 .114 0 0
.299 .587 .114 0 0
"
/>
<feComponentTransfer
// Use sRGB instead of linearRGB to be consistent with how CSS gradients work.
colorInterpolationFilters="sRGB"
>
<feFuncR
type="table"
tableValues={ values.r.join( ' ' ) }
/>
<feFuncG
type="table"
tableValues={ values.g.join( ' ' ) }
/>
<feFuncB
type="table"
tableValues={ values.b.join( ' ' ) }
/>
<feComponentTransfer
// Use sRGB instead of linearRGB to be consistent with how CSS gradients work.
colorInterpolationFilters="sRGB"
>
<feFuncR
type="table"
tableValues={ values.r.join( ' ' ) }
/>
<feFuncG
type="table"
tableValues={ values.g.join( ' ' ) }
/>
<feFuncB
type="table"
tableValues={ values.b.join( ' ' ) }
/>
<feFuncA
type="table"
tableValues={ values.a.join( ' ' ) }
/>
</feComponentTransfer>
<feComposite
// Re-mask the image with the original transparency since the feColorMatrix above loses that information.
in2="SourceGraphic"
operator="in"
<feFuncA
type="table"
tableValues={ values.a.join( ' ' ) }
/>
</filter>
</defs>
</SVG>
<style dangerouslySetInnerHTML={ { __html: stylesheet } } />
</feComponentTransfer>
<feComposite
// Re-mask the image with the original transparency since the feColorMatrix above loses that information.
in2="SourceGraphic"
operator="in"
/>
</filter>
</defs>
</SVG>
);
}

/**
* SVG and stylesheet needed for rendering the duotone filter.
*
* @param {Object} props Duotone props.
* @param {string} props.selector Selector to apply the filter to.
* @param {string} props.id Unique id for this duotone filter.
* @param {Values} props.values R, G, B, and A values to filter with.
*
* @return {WPElement} Duotone element.
*/
function InlineDuotone( { selector, id, values } ) {
return (
<>
<DuotoneFilter id={ id } values={ values } />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand we need this for the block-instance duotone. Though, in the site editor, we'll be adding the SVG when we already have it, don't we? This is all the ways we add SVGs in the site editor:

  • We load them all via the in_admin_header hook (see). These aren't in the scope of the iframed editor. So perhaps we can add there a check for not loading them if we're in the site editor?
  • We load them all dynamically via the regular preset mechanism introduced in this PR (svgFilters).
  • We load the specific filter when the user attaches it to a block instance.

I don't think this is a blocker given the time-sensitive situation we're in. Thought I'd think it merits a follow-up PR to organize it a bit.

<DuotoneStylesheet id={ id } selector={ selector } />
</>
);
}
Expand Down Expand Up @@ -321,7 +348,7 @@ const withDuotoneStyles = createHigherOrderComponent(
<>
{ element &&
createPortal(
<DuotoneFilter
<InlineDuotone
selector={ selectorsGroup }
id={ id }
values={ getValuesFromColors( values ) }
Expand All @@ -335,6 +362,15 @@ const withDuotoneStyles = createHigherOrderComponent(
'withDuotoneStyles'
);

export function PresetDuotoneFilter( { preset } ) {
return (
<DuotoneFilter
id={ `wp-duotone-${ preset.slug }` }
values={ getValuesFromColors( preset.colors ) }
/>
);
}

addFilter(
'blocks.registerBlockType',
'core/editor/duotone/add-attributes',
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ export { getBorderClassesAndStyles, useBorderProps } from './use-border-props';
export { getColorClassesAndStyles, useColorProps } from './use-color-props';
export { getSpacingClassesAndStyles } from './use-spacing-props';
export { useCachedTruthy } from './use-cached-truthy';
export { PresetDuotoneFilter } from './duotone';
1 change: 1 addition & 0 deletions packages/block-editor/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import './hooks';
export {
PresetDuotoneFilter as __unstablePresetDuotoneFilter,
getBorderClassesAndStyles as __experimentalGetBorderClassesAndStyles,
useBorderProps as __experimentalUseBorderProps,
getColorClassesAndStyles as __experimentalGetColorClassesAndStyles,
Expand Down
4 changes: 4 additions & 0 deletions packages/blocks/src/api/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ export const __EXPERIMENTAL_STYLE_PROPERTY = {
support: [ 'color', 'text' ],
requiresOptOut: true,
},
filter: {
value: [ 'filter', 'duotone' ],
support: [ 'color', '__experimentalDuotone' ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really need declare support, do we? I mean, it's fine and even nice because it's whole. For a moment I thought whether this would make any block panel in the GS sidebar to show (despite not having any other support), but it seems we're fine there. Nothing to do here, just documenting my thought process and worries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually the cause of the block instance issue #37727 (comment). It's adding the filter to the block container which we don't want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove the filter constant, the duotone ruleset (editor-styles-wrapper .wp-block-image img { filter: <value>; }) is not rendered client-side in the site editor as the other block styles (we could also remove the filter in pickStyleKeys).

I've checked the plugin with the following versions:

  • WordPress latest: this works fine.
  • WordPress 5.9: this works fine.
  • WordPress 5.8: this does not work.

Why does it work in 5.9 and onwards?

It seems we enqueue some extra stylesheet starting in 5.9. I don't know why or when we've introduced this, but we shouldn't rely on it in the site editor, as it's redundant and it should be removed. Can we make the duotone rendering in the site editor work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#40185 is related although probably don't fully solve the issue (need to test more)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

40185 has landed and fixes this for 5.9. In 6.0 we need to get WordPress/wordpress-develop#2542 in for the extra global stylesheet not being enqueued.

},
linkColor: {
value: [ 'elements', 'link', 'color', 'text' ],
support: [ 'color', 'link' ],
Expand Down
8 changes: 6 additions & 2 deletions packages/edit-site/src/components/block-editor/resizable-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const HANDLE_STYLES_OVERRIDE = {
left: undefined,
};

function ResizableEditor( { enableResizing, settings, ...props } ) {
function ResizableEditor( { enableResizing, settings, children, ...props } ) {
const deviceType = useSelect(
( select ) =>
select( editSiteStore ).__experimentalGetPreviewDeviceType(),
Expand Down Expand Up @@ -182,7 +182,11 @@ function ResizableEditor( { enableResizing, settings, ...props } ) {
name="editor-canvas"
className="edit-site-visual-editor__editor-canvas"
{ ...props }
/>
>
{ /* Filters need to be rendered before children to avoid Safari rendering issues. */ }
{ settings.svgFilters }
ajlende marked this conversation as resolved.
Show resolved Hide resolved
{ children }
</Iframe>
</ResizableBox>
);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/edit-site/src/components/editor/global-styles-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { store as editSiteStore } from '../../store';
import { useGlobalStylesOutput } from '../global-styles/use-global-styles-output';

function useGlobalStylesRenderer() {
const [ styles, settings ] = useGlobalStylesOutput();
const [ styles, settings, svgFilters ] = useGlobalStylesOutput();
const { getSettings } = useSelect( editSiteStore );
const { updateSettings } = useDispatch( editSiteStore );

Expand All @@ -37,6 +37,7 @@ function useGlobalStylesRenderer() {
updateSettings( {
...currentStoreSettings,
styles: [ ...nonGlobalStyles, ...styles ],
svgFilters,
__experimentalFeatures: settings,
} );
}, [ styles, settings ] );
Expand Down
Loading