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

Update empty theme to use Gutenberg alignments #233

Merged
merged 5 commits into from
Mar 19, 2021
Merged

Conversation

scruffian
Copy link
Collaborator

Now that WordPress/gutenberg#29335 has merged, we can update empty theme to use the new layout styles.

I'm not sure if I've done this right, the group block looks like this:

localhost_4759__p=443

@scruffian scruffian added the block-based theme A theme using HTML templates label Mar 18, 2021
@scruffian scruffian self-assigned this Mar 18, 2021
}
"layout": {
"contentSize": "800px",
"wideSize": "1000px"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should match the widths used above, right? 840px and 1100px?

@kjellr
Copy link
Collaborator

kjellr commented Mar 19, 2021

This is what I'm seeing when testing against the latest Gutenberg trunk branch (which includes the change in WordPress/gutenberg#29335).

  • The post/page editor looks correct.
  • All Site Editor items that are not inside of a group block appear to take up the full page width, regardless of their alignment setting. For items that are inside of a group block, they're all 800px wide, regardless of their width setting. 🤔
  • The front-end mirrors the Site editor.

Post/Page Editor:

Screen Shot 2021-03-19 at 10 17 47 AM

Site Editor:

Screen Shot 2021-03-19 at 10 17 04 AM
Front End:

Screen Shot 2021-03-19 at 10 17 27 AM

@scruffian
Copy link
Collaborator Author

@kjellr I just pushed another update to this to take advantage of the change in WordPress/gutenberg#29335.

Let me know how it looks now...

@kjellr
Copy link
Collaborator

kjellr commented Mar 19, 2021

That definitely helped, but this is still an issue:

All Site Editor items that are not inside of a group block appear to take up the full page width, regardless of their alignment setting.

I think we need those global layout rules to apply to blocks in the root of the page, and also inside of template parts, right?

Also, I'm seeing some weirdness with standard-width text blocks inside of Group blocks. This only happens in the post/page editor:

Screen Shot 2021-03-19 at 12 44 46 PM

@youknowriad
Copy link
Contributor

I think we need those global layout rules to apply to blocks in the root of the page, and also inside of template parts, right?

Depending on the design of the theme, this might not be true. It's only true for themes with regular layouts. A header template part followed by post content followed by a footer and even in this case, you might want your footer to be full width or your header...

I think the current behavior is the right one (opt-in to global layout in templates where you need it). We can continue discussing here as it's related.

WordPress/gutenberg#29983

@youknowriad
Copy link
Contributor

I checked the paragraph issue, it seems related to the "reset" we apply on the editor styles, I need to think about it more but it's more a Gutenberg issue. I think this PR is good.

@youknowriad
Copy link
Contributor

@kjellr This PR should fix the paragraph thing, but I'd love some extensive tests WordPress/gutenberg#30038

@kjellr
Copy link
Collaborator

kjellr commented Mar 19, 2021

I think the current behavior is the right one (opt-in to global layout in templates where you need it). We can continue discussing here as it's related.

I'll need to think about that a little. I worry that it might be a bit of a complicated thing for users to grasp: in the page editor, when you add a paragraph block it looks one way (centered in the page canvas with a max-width), but if you add that same block to the root of the global page canvas, it takes up the full width of the page and there's no clear way to change that. My gut says it'll be more intuitive to have the same behavior in both places by default.

Site Editor Post Editor
Screen Shot 2021-03-19 at 1 12 35 PM Screen Shot 2021-03-19 at 1 13 11 PM

@youknowriad
Copy link
Contributor

The problem is the post editor has a container block which is "post-content" but that container is external to the editor, it's in the template, so we need a way to tell the editor what layout is actually defined on the template, does that container support wide/full widths, what are the sizes...

In the site editor, you're rendering the "body" of the page by default, which means there's no layout by default, there's no container and each theme could have its own layout, its own design, so there's no way to say: there's one layout use it everywhere.

Imagine a theme doing this for instance

example layout

@kjellr
Copy link
Collaborator

kjellr commented Mar 19, 2021

I think that layout would be possible either way. The benefit to setting default layout widths on the body (while allowing users/themers to opt-out of it) is just the consistency aspect: a paragraph would look the same in both places.
It makes building a page a lot more like building a page in Gutenberg today, and I think that makes sense.

I'll think more about it over the weekend, and loop in some other designers — I think this could benefit from some wider consideration.


Also, either way I think this needs some adjustment: template parts that sit directly in the root of the document have alignment controls, but they don't really do much today:

template-part

@youknowriad
Copy link
Contributor

Also, either way I think this needs some adjustment: template parts that sit directly in the root of the document have alignment controls, but they don't really do much today:

Yeah, this sounds like a bug to me, I'd personally just remove all alignments from root level blocks on the site editor (templates), potentially keep left/right but not sure.

Copy link
Collaborator

@kjellr kjellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the PR looks good! I'll go ahead and merge this in.

@kjellr kjellr merged commit e6ba1eb into master Mar 19, 2021
@kjellr kjellr deleted the update/emptytheme branch March 19, 2021 19:14
@scruffian
Copy link
Collaborator Author

I think if we add inherit layout support to template parts that would go a long way to fixing these issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
block-based theme A theme using HTML templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants