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

Jump to bottom

Style variations preview is cut off #41177

Closed
annezazu opened this issue May 20, 2022 · 11 comments · Fixed by #41198
Closed

Style variations preview is cut off #41177

annezazu opened this issue May 20, 2022 · 11 comments · Fixed by #41198
Assignees
@Mamaduka
Labels
[Feature] Theme Style Variations Related to style variations provided by block themes Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

Description

When using 6.0 RC 3, the style variations preview is now cut off. If I use GB 13.2.2, it shows up correctly so something must have changed recently. cc @ndiego as a heads up & @glendaviesnz since you chimed in on another similar issue @gziolo as co-core editor tech lead.

Here's another similar issue about the preview where you can see how this should appear #40699

Step-by-step reproduction instructions

  1. Open the Appearance > editor
  2. Open styles
  3. Select "browse variations"
  4. Notice problem

Screenshots, screen recording, code snippet

What it looks like:
Screen Shot 2022-05-19 at 6 16 44 PM

What it should look like:
165822972-5c946f50-edad-48fc-b3d8-cb9e23bc7771

Environment info

  • WordPress 6.0 RC 3
  • No GB
  • TT2

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes
@annezazu annezazu added [Type] Bug An existing feature does not function as intended [Feature] Theme Style Variations Related to style variations provided by block themes Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels May 20, 2022
@gziolo gziolo moved this from Triage to Todo in WordPress 6.0 Editor Tasks May 20, 2022
@getdave
Copy link
Contributor

getdave commented May 20, 2022

I remember #40842 was adjusting things to do with iframes and styles.

@Mamaduka
Copy link
Member

I think I was able to track down the problem. It's similar to the issue with the Pattern previews in WP 5.9.

When the SCRIPT_DEBUG is set to false and Gutenberg plugin isn't active, some styles are leaking into the iframes. In this case, min-width is set for the body.

The main problem with this bug is that it's tough to test with Gutenberg active.

I will try to create a POC patch.

Screenshot

CleanShot 2022-05-20 at 14 56 57

@ndiego
Copy link
Member

ndiego commented May 20, 2022 •
edited
Loading

I can confirm. Looks like a lot more styles are being applied to the <body> tag in RC3, and one of them is min-width: 240px.

RC3 only
image

RC3 with Gutenberg 13.2.2
image

These styles look to be applied straight to all <body> tags in the editor.

We could fix this with something like the following. It is not very elegant but would be the quickest solution that could then be iterated on post-release.

body.editor-styles-wrapper {
    min-width: 0;
}

@ndiego
Copy link
Member

ndiego commented May 20, 2022 •
edited
Loading

@Mamaduka I just confirmed that adding the following to common.min.css does fix the problem. Not a great solution, but one nonetheless 🤷‍♂️

@media screen and (max-width: 782px) {
    body.editor-styles-wrapper {
        min-width: 0;
    }
}

@Mamaduka
Copy link
Member

@ndiego, this is what I was thinking:

packages/edit-site/src/components/global-styles/preview.js

<EditorStyles
	styles={ [
		...styles,
		{
			css: 'body{min-width: 0;}',
			isGlobalStyles: true,
		},
	] }
/>

@youknowriad, any preferences on we should add min-width:0?

@ndiego
Copy link
Member

ndiego commented May 20, 2022

Yeah, that is a bit more elegant that my hack 🙌

@youknowriad
Copy link
Contributor

youknowriad commented May 20, 2022 •
edited
Loading

I'm fine adding the min width to the iframe styles but let's ensure that it's added before the styles that come from the theme (themes can potentially override this right?)

Also it's probably something that should only be added in the iframe, and not for all usage of EditorStyles

@youknowriad
Copy link
Contributor

The suggested fix above applies to the previews only, I assume these styles can leak to the main iframe too right? Don't we have any issues there?

@Mamaduka
Copy link
Member

Most of the other iframes are larger than 240px. The better fix would be to avoid leaking these styles in iframes, but I'm not sure how :(

@youknowriad
Copy link
Contributor

Yes, let's just add it to the iframe component with a comment now. I don't think we can easily avoid the leaks without removing the compact CSS thing which we can't do.

The other thing would be to figure out to split styles into two calls instead of one in Core depending on whether they're theme styles or core styles.

@Mamaduka Mamaduka mentioned this issue May 20, 2022
Merged
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 20, 2022
@gziolo gziolo removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 20, 2022
@gziolo gziolo added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 20, 2022
@gziolo gziolo moved this from Triage to In Progress in WordPress 6.0 Editor Tasks May 20, 2022
Repository owner moved this from In Progress to Done in WordPress 6.0 Editor Tasks May 20, 2022
@annezazu
Copy link
Contributor Author

Thank you all for jumping in and resolving this so quickly.

@ndiego ndiego removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 26, 2022
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Mamaduka Mamaduka

Labels
[Feature] Theme Style Variations Related to style variations provided by block themes Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
Projects
No open projects
WordPress 6.0 Editor Tasks
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Site Editor: Set min-width for styles preview WordPress/gutenberg
7 participants
@Mamaduka @youknowriad @getdave @gziolo @ndiego @annezazu @priethor