WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.3k stars 4.11k forks source link

Erratic padding in pattern previews #52421

Open jameskoster opened 1 year ago

jameskoster commented 1 year ago

This seems to be more of an issue with some themes than others, so a bit tricky to track down. For testing purposes Twenty Twenty-Two is a good theme to have active.

Pattern previews (in the Inserter, and the Patterns section of the Site Editor) often include padding that is not visible when the pattern is inserted.

Notice the previews here all have empty space on the left and right:

Screenshot 2023-07-07 at 13 59 00

But when you insert one, there is no such padding:

Screenshot 2023-07-07 at 13 59 19

It is also not present if you edit the pattern in focus mode:

Screenshot 2023-07-07 at 14 08 42
Mamaduka commented 1 year ago

It could be related to #52155.

Mamaduka commented 1 year ago

@jameskoster, can you re-test this now that #52640 is merged?

jameskoster commented 1 year ago

Still an issue I'm afraid. Patterns previews always seem to entertain the global padding setting. I suppose this could be related to https://github.com/WordPress/gutenberg/issues/51937.

Mamaduka commented 1 year ago

Let's double-check and close one if they have the same cause.

P.S. The #51937 was punted to WP 6.4.

glendaviesnz commented 1 year ago

The padding in the inserter matches the padding in the editor when I test this with TT2 patterns. It is only visible in the inserter because of the grey background.

Screenshot 2023-07-19 at 3 56 00 PM

this is the padding that TT2 specifies for the root container:

Screenshot 2023-07-19 at 3 50 39 PM

It might be tricky to get around this as in the case of TT2 this is coming from a static style.css file, not from global styles, so would be difficult to remove this from the preview iframe as the patterns could be reliant on other styles in that file - but maybe we can add a higher specificity style to the iframe to override an root container padding set by the theme 🤔

glendaviesnz commented 1 year ago

So, this can be fixed by setting padding to 0 on the iframe with an ! important, but this also does make some patterns look strange, eg. the first and last ones below end up with text hard against the edge:

Screenshot 2023-07-19 at 4 44 51 PM

which is the worse of these two evils?

jameskoster commented 1 year ago

Imo the goal here should be for the previews to match the result upon insertion as closely as possible. When details in the preview do not match the result it can be a bit confusing.

So I'd say the first and last previews in the screenshot above are working as intended (assuming neither have any padding values on their root containers). Presenting them this way helps the user understand that the pattern will fill the container upon insertion.

glendaviesnz commented 1 year ago

I put up a PR for this, but there is an issue with removing this padding affecting blocks that use it to calculate margins for setting full width display.

tellthemachines commented 1 year ago

Moving to 6.4 as there is no straightforward solution to this and any changes to padding behaviour may have unexpected side-effects, so we need a little more time to work something out!

jameskoster commented 1 year ago

@glendaviesnz just to connect a dot, is this technically the same issue as https://github.com/WordPress/gutenberg/issues/51937?

Happy to close this out and consolidate if so.

tellthemachines commented 11 months ago

@glendaviesnz just to connect a dot, is this technically the same issue as #51937?

This is not quite the same issue, given that the fix for #51937 hasn't fixed this one.

Can you point to any other themes aside from TT2 where this happens? Asking because that padding in the specific case of TT2 is actually coming from the theme stylesheet. Because the previews are iframed, there is no reliable way of targeting that element for an override. I'm not sure it would be desirable to override theme styles anyway 😅

jameskoster commented 10 months ago

Thanks for digging that up. I suppose this issue will occur whenever the active theme adds a custom padding value to body or similar.

I'm not sure it would be desirable to override theme styles anyway

In this case I think it would be because the preview is (supposed to be) the pattern in isolation. Since the padding is not part of the pattern it breaks that convention leaving the user unsure whether it's a margin value attached to the pattern, or something else. Still, if there's no reliable way to override then I guess there's not much we can do.

tellthemachines commented 10 months ago

I'm not entirely sure but it may be possible to inject styles into the iframe from our side 🤔

Would it be safe to assume that in the pattern preview we'd always want the body to have zero margin/padding?

jameskoster commented 10 months ago

Would it be safe to assume that in the pattern preview we'd always want the body to have zero margin/padding?

Imo yes. Otherwise its unclear whether that empty space in the preview is part of the pattern or something else. Ideally what you see upon insertion matches the preview as closely as possible.

richtabor commented 1 month ago

This is no longer happening.

Image

jameskoster commented 1 month ago

Unfortunately it does still happen. It occurs when a theme adds padding to the body.

The preview includes the body padding, which suggests it's part of the pattern. But when you insert the pattern there is no such padding.

Screenshot 2024-07-24 at 16 16 02
richtabor commented 1 month ago

It occurs when a theme adds padding to the body.

There must be another variable at play. In TT4 for example, there is outer padding applied to the body, but not to the patterns.

CleanShot 2024-07-24 at 12 35 49