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.

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

Layouts: Block previews contain unexpected amount of blank space #39355

Closed
ianstewart opened this issue Feb 10, 2020 · 15 comments
Closed

Layouts: Block previews contain unexpected amount of blank space #39355

ianstewart opened this issue Feb 10, 2020 · 15 comments
Labels
[Feature Group] Appearance & Themes Features related to the appearance of sites. [Type] Enhancement

Comments

@ianstewart
Copy link
Contributor

Many of the previews in our Layout Selector have unexpected gaps in the preview that do not reflect the final output.

For example this About template …

image

Compared to the final output on the front-end of the site …

image

Enhancement: Provide a preview that matches the expected front-end result to improve the ability for users to select the right layout design for their needs and to better match.

@marekhrabe marekhrabe changed the title Layouts: Provide previews that reflect the output of the front end. Layouts: Block previews contain unexpected amount of blank space Feb 25, 2020
@marekhrabe
Copy link
Contributor

Possibly solved by the iframe preview, as media queries will have a better match

@jeryj
Copy link
Contributor

jeryj commented Feb 27, 2020

I've dug into this a bit to see if the iframe PR (#39628) will fix this.

  • The iframe PR preview matches the resulting editor view extremely closely on almost all templates (the ones that don't will get new spin-off issues).
  • Most of the discrepancies between the preview and front-end are also issues between the editor and front-end.

That said, here's what I've found to be the specific issue with the extra blank spaces:

  • This is an issue focused with Varia and Varia child themes (which is a lot of them). They have this CSS on the front-end:
.wp-block-spacer {
   display: block;
   margin-bottom: 0 !important;
   margin-top: 0 !important;
}
  • The Gutenberg spacer block in the editor does not have the wp-block-spacer class.
  • I would think it is standard for all Gutenberg blocks to have a wp-block-[blockname] class attached to them. It appears that many do. If this is true, then I'd recommend we open an issue in Gutenberg to have that classname added to the spacer block in the editor.

@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

@jeryj Thanks for your research here. Can I confirm the following:

  1. That this isn't a SPT specific issue.
  2. That the "fix" is actually due to a missing class attribute on the saved core/spacer Block in Gutenberg Core?

If so then I'd suggest this issue is actually better raised:

  • on the Varia Theme repo - regarding what you've learned above.
  • on the Gutenberg repo - solely regarding the missing class (but without all the info above as it's not necessarily relevant to Core).

Thanks again

@jeryj
Copy link
Contributor

jeryj commented Mar 2, 2020

  1. This is not a SPT specific issue. The extra-large spacing is also happening on the editor of (likely) all themes and is happening on the front-end of themes like 2020.

Here's a screenshot of 2020 with the about page template on the front-end. You can see the large spacing is present:
Screen Shot 2020-03-02 at 8 37 36 AM

  1. No, that doesn't seem to be the actual fix. Adding the wp-block-spacer class doesn't remove the margins. This led me to looking into the issue further.

The 2020 theme above on the front-end also does not contain margin-bottom/top on the spacer, but still has large gaps. The issue there is that 2020 applies margins to groups and Varia based themes must have a spacer between groups.

Here's a screenshot of Varia on the front-end with the about page template when the spacers are removed from the template:

Screen Shot 2020-03-02 at 8 41 25 AM

Compare this same page (spacers removed) to the 2020 front-end. Spacing looks good when spacers are entirely removed, since the margins are handled by the groupings automatically:
Screen Shot 2020-03-02 at 10 01 10 AM

So we have a couple issues we're fixing for now:

  1. Different themes handle group spacing differently. Some need spacers, some don't. My quick take on this is that spacers should not need to be added to get appropriate default spacing. That would mean a CSS change on Varia and varia based themes, if we went that route.
  2. If we do the above, then the spacers would need to be removed from the default templates.
  3. I think the margins on the editor around the spacer (32px top/bottom) may be for appropriate spacing to allow the appender + to be easier to access. In this sense, I think we should set the top/bottom margins to 0 on spacers in the SPT preview). However, this will still show a large space.

Here's a screenshot of the spacer top/bottom margins set to 0 in the SPT preview:
Screen Shot 2020-03-02 at 9 56 07 AM

Still not so great :/

Here's a screenshot with the spacers removed entirely from DOM in the SPT preview:
Screen Shot 2020-03-02 at 9 56 23 AM

Now it looks right 👍

So, in the end, it may be best to change the about template to not use spacers.

It may open a can of worms, but it may be good to also fix Varia based theme CSS so that spacers are not needed to get a nice looking template layout when using grouped blocks.

cc @getdave

@jeryj
Copy link
Contributor

jeryj commented Mar 2, 2020

Quick follow up. I had previously said:

I think we should set the top/bottom margins to 0 on spacers in the SPT preview)

I tried this, and, because of collapsing margins, setting the margins to 0 does not actually do anything. The previews still have the same amount of spacing.

@getdave
Copy link
Contributor

getdave commented Mar 2, 2020

Thanks @jeryj . Interesting I've been working on trying to get dimensions (spacing) controls added to the Group Block in Core for a while now with little success. The main issue seems to be we can't decide how it should work on Themes that rely on spacer blocks.

@obenland
Copy link
Member

obenland commented Mar 2, 2020

@ianstewart @iamtakashi What's your preferred way to handle this?

@iamtakashi
Copy link
Contributor

iamtakashi commented Mar 3, 2020

In terms of vertical spacing, I don't see drastic differences between what I see in the preview and the editor with Varia based themes.

Screenshot 2020-03-03 at 11 30 41

In my opinion, the editor should show the content as close as possible to the front-end of the site, and I think that the editor styles in Varia based themes need to be improved.

@iamtakashi
Copy link
Contributor

There is a Varia based theme issue that should make this situation better. (it won't still make 1:1, but better.)

@iamtakashi
Copy link
Contributor

There is a Varia based theme issue that should make this situation better

Before and after

Screenshot 2020-03-03 at 17 14 12

@jeryj
Copy link
Contributor

jeryj commented Mar 3, 2020

I think those Varia changes will help, but I think a change to the master template would be a good as well. My reason for recommending a template change is that Twenty Twenty experiences a similar large spacing, but it's consistent across preview, editor, and front-end:

Preview
About template preview

Editor
About template  editor view

Front-end
About template front-end

Because of this, it seems like it'd be good to tweak the master template to display more consistently across Twenty Twenty and Varia-based themes.

cc @iamtakashi @ianstewart

@iamtakashi
Copy link
Contributor

it seems like it'd be good to tweak the master template to display more consistently across Twenty Twenty and Varia-based themes.

That's ideal, but unfortunately, it's impossible to make the layouts work consistently in both Twenty Twenty and Varia-based themes as they handle spacing differently, and it's going to be disruptive for all the existing sites to change how Varia based themes handle spacing like Twenty Twenty.

This problem illustrates one of the reasons why we should move on to offering Block patterns — more smaller chunks of a page, and let the user decide the spacing between the patterns.

@obenland
Copy link
Member

obenland commented Mar 5, 2020

This problem illustrates one of the reasons why we should move on to offering Block patterns

@ianstewart @iamtakashi In the meantime, how should this issue be handled?

@iamtakashi
Copy link
Contributor

iamtakashi commented Mar 9, 2020

I think, at this point, this issue is more of Varia's editor-style issue, not a preview issue because the preview matches with the editor.

@ianstewart
Copy link
Contributor Author

I'll close this one and we can move it to a Themes discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature Group] Appearance & Themes Features related to the appearance of sites. [Type] Enhancement
Projects
None yet
Development

No branches or pull requests

6 participants