Iframe block, pattern and template previews #28165
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
|
Size Change: +342 B (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
ellatrix
force-pushed
the
try/iframe-preview
branch
from
e5cb7b4
to
ef9dbef
Compare
ellatrix
referenced
this pull request
Jul 8, 2021
ellatrix
force-pushed
the
try/iframe-preview
branch
from
084774d
to
5af0c2b
Compare
ellatrix
requested review from
ajitbohra,
spacedmonkey,
TimothyBJacobs,
nerrad and
ntwb
as code owners
ellatrix
force-pushed
the
try/iframe-preview
branch
from
02f68f1
to
d775b34
Compare
ellatrix
commented
Jul 11, 2021
| @@ -22,6 +22,7 @@ export const settings = { | |||
| icon, | |||
| variations, | |||
| example: { | |||
| viewportWidth: 600, // Columns collapse "@media (max-width: 599px)". | |||
There was a problem hiding this comment.
Note that this is an existing API. The viewportWidth property already exists. Previously we didn't need to set if for these blocks, because the preview viewport would be the same as the main parent window. A lot of previews are setting the default viewport width to 500. Maybe we should change the default to be a bit larger.
ellatrix
commented
Jul 11, 2021
| @@ -18,6 +18,10 @@ import { | |||
| // eslint-disable-next-line no-restricted-imports | |||
| import { find } from 'puppeteer-testing-library'; | |||
|
|
|||
| const twentyTwentyError = `Stylesheet twentytwenty-block-editor-styles-css was not properly added. | |||
| For blocks, use the block API's style (https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/#style) or editorStyle (https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/#editor-style). | |||
| For themes, use add_editor_style (https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-support/#editor-styles).`; | |||
There was a problem hiding this comment.
The 2020 theme is not adding editor styles through add_editor_style. For some reason the widgets tests are using this theme. It's outside the scope of this PR to fix this.
ellatrix
commented
Jul 11, 2021
| @@ -109,7 +109,7 @@ function Editor( { | |||
|
|
|||
| const editorSettings = useMemo( () => { | |||
| const result = { | |||
| ...omit( settings, [ 'styles' ] ), | |||
There was a problem hiding this comment.
The omission was added in https://github.com/WordPress/gutenberg/pull/27947/files#diff-d00cfe048041886ee9b16f6f42422317b6e770cbea20cbe0e4d0ca7c8e0ec0e1R102. It's not clear to me why at this point in the code. It prevents the block editor from being able to access this setting. Cc @youknowriad.
ellatrix
changed the title
ellatrix
changed the title
ellatrix
force-pushed
the
try/iframe-preview
branch
from
210a123
to
be650ce
Compare
There was a problem hiding this comment.
Things worked well on my tests and this PR fixes both issues with template previews referred on #33825 👍
I think we just need to clarify if the changes in packages/edit-post/src/editor.js are safe and we can merge this PR.
| @@ -20,6 +20,7 @@ export { metadata, name }; | |||
| export const settings = { | |||
| icon, | |||
| example: { | |||
| viewportWidth: 601, // Columns collapse "@media (max-width: 600px)". | |||
There was a problem hiding this comment.
I think if we want we can avoid adding a new viewPortWidth setting by using isStackedOnMobile false attributes on columns and media-text blocks.
There was a problem hiding this comment.
I'm not sure if that's the best approach though. Why do we set previews to such a small width by default?
There was a problem hiding this comment.
Good question, I'm not sure why the previews are so small by default. I guess it may make sense to explore changing the default.
|
Main blocker here is getting the tests to pass. Looking into that again. |
ellatrix
force-pushed
the
try/iframe-preview
branch
from
be650ce
to
f2f5583
Compare
|
Test are now passing. Would be good to have another review :) |
ellatrix
force-pushed
the
try/iframe-preview
branch
from
2455d59
to
bc50888
Compare
jorgefilipecosta
approved these changes
Aug 17, 2021
| @@ -20,6 +20,7 @@ export { metadata, name }; | |||
| export const settings = { | |||
| icon, | |||
| example: { | |||
| viewportWidth: 601, // Columns collapse "@media (max-width: 600px)". | |||
There was a problem hiding this comment.
Good question, I'm not sure why the previews are so small by default. I guess it may make sense to explore changing the default.
|
Thanks @jorgefilipecosta! |
|
🎉 |
annezazu
added
[Type] Enhancement
|
@ellatrix do you know if this is going to make it into 5.9? I am experiencing an issue where pattern previews are not fully respecting theme styles in core, but work fine with Gutenberg installed. I believe this PR fixes the issue, which is why everything is working in Gutenberg, so just curious when this will make it into core. The latest 5.9 nightly does not seem to have it. |
|
Yes, this should land in WP 5.9. |
|
Thanks for the followup @ellatrix. I am struggling to get the correct fonts to show up in the previews. My theme is using a Google font. Do you know offhand if this is currently possible? The font is currently being enqueued using |
youknowriad
reviewed
Nov 15, 2021
| left: __experimentalPadding, | ||
| right: __experimentalPadding, | ||
| top: __experimentalPadding, | ||
| height: contentHeight * scale, |
There was a problem hiding this comment.
There's a weird behavior that is happening now in the patterns explorer because of this component (I think). When you switch to the "buttons" then the "columns" category tab in the modal patterns explorer, you'll notice that the patterns using columns (for example the three columns with images ones) jump between a full width image into the real output (kind of like moving from small screens to bigger) when first rendering. I tried solving this issue but failure for the moment, I'm not sure yet why it's rendering first like if it wasn't scaled (small screen) and then when scaled down, it shows the right sizes...
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.
Part of #33346
Description
This seems like the next logical step for iframing block lists.
How has this been tested?
Screenshots
Types of changes
Checklist: