Duotone: Pass filters to the Post Editor #49239
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
scruffian
commented
Mar 21, 2023
scruffian
commented
Mar 21, 2023
|
Size Change: +1.44 kB (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in a0be2bf. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4514581517
|
scruffian
commented
Mar 22, 2023
ajlende
reviewed
Mar 22, 2023
Comment on lines
97
to
105
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 0 0" | ||
| width="0" | ||
| height="0" | ||
| focusable="false" | ||
| role="none" | ||
| dangerouslySetInnerHTML={ { __html: transformedSvgs } } | ||
| /> |
There was a problem hiding this comment.
Since we're adding all this front matter in this case and we're outputting them all at once in the frontend too, there's another performance improvement that we can make with duotone filters by wrapping all the filter definitions in a single SVG wrapper like this.
Not for this PR, but as a follow-up.
There was a problem hiding this comment.
Why would that be more performant?
There was a problem hiding this comment.
Re-reading what I wrote, I really didn't explain that well. What I'm saying is that we can have one wrapper for all the SVG defs, so transformedSvgs would just contain the definitions.
<filter id="wp-duotone-grayscale">
<feColorMatrix ... />
<feComponentTransfer ...>...</feComponentTransfer>
<feComposite ... />
</filter>
<filter id="wp-duotone-midnight">
...
</filter>
...And the final output would look like this.
<svg xmlns="http://www.w3.org/2000/svg" ... />
<defs>
<filter id="wp-duotone-grayscale">
<feColorMatrix ... />
<feComponentTransfer ...>...</feComponentTransfer>
<feComposite ... />
</filter>
<filter id="wp-duotone-midnight">
...
</filter>
...
</defs>
</svg>Instead of what we currently have in this PR.
<svg xmlns="http://www.w3.org/2000/svg" ... />
<svg xmlns="http://www.w3.org/2000/svg" ... />
<defs>
<filter id="wp-duotone-grayscale">
<feColorMatrix ... />
<feComponentTransfer ...>...</feComponentTransfer>
<feComposite ... />
</filter>
</defs>
</svg>
<svg xmlns="http://www.w3.org/2000/svg" ... />
<defs>
<filter id="wp-duotone-midnight">
...
</filter>
</defs>
</svg>
...
</svg>In the frontend, doing something similar could maybe shave off another kilobyte if there are a lot of filters on the page.
ajlende
reviewed
Mar 22, 2023
scruffian
commented
Mar 22, 2023
scruffian
commented
Mar 22, 2023
ajlende
reviewed
Mar 23, 2023
packages/edit-site/src/components/block-editor/editor-canvas.js
Outdated
Show resolved
Hide resolved
|
I've made all of the requested changes, but I ended up reverting the change using a portal to render the |
This was referenced Mar 24, 2023
|
Thanks for all the work, this is looking good to me. Should we also update the output of |
ellatrix
reviewed
Mar 24, 2023
|
@ellatrix Because for me, these are editor styles. What would we want to separate some editor styles rendered in one component, while the others rendered by something else? Rendering in head or body is an implementation detail for me. I understand that it creates friction here but if we really want to keep the styles in head and svgs in body, I think it should still be possible to do within the same component. |
added 2 commits
March 24, 2023 13:51
… SVG elements in filterSvgs
ajlende
reviewed
Mar 24, 2023
packages/block-editor/src/components/global-styles/use-global-styles-output.js
Show resolved
Hide resolved
ajlende
reviewed
Mar 24, 2023
packages/block-editor/src/components/global-styles/use-global-styles-output.js
Show resolved
Hide resolved
My mistake for missing that—you mentioned it earlier. Fixed in 0dabaf4. |
2 tasks
youknowriad
approved these changes
Mar 29, 2023
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Resolves the issue where global block style duotone filters were not applied in the post editor.
Why?
Fixes #48586
How?
<EditorStyles>into the<body>of the post editor iframe because SVG elements are not valid children of<head>.<EditorStyles>to handlestyleswith__unstableType: 'svgs'duotoneto populate thestyleswith SVGs and CSS custom properties.Testing Instructions
{ "styles": { "blocks": { "core/image": { "filter": { "duotone": "var(--wp--preset--duotone--midnight)" } } } } }Testing Instructions for Keyboard
Screenshots or screencast
Before (post editor)
After (post editor)
Co-authored-by: Alex Lende 5129775+ajlende@users.noreply.github.com