Ensure bundled query patterns inherit correct layout settings #30506
Conversation
kjellr
added
[Type] Bug
|
Size Change: 0 B Total Size: 1.42 MB ℹ️ View Unchanged
|
|
Thank you for the PR! I'm conflicted on this one:
So it boils down to: to group or not to group. I'm always a fan of fewer containers if we can do it. Question: when you build this yourself, couldn't you just manually insert the group first, make it inherit the layout, and then insert the query inside? Or does the group need to be nested inside the query? I'm leaning towards not wanting to add the additional group, but this is not a strong opinion, and I'd gladly defer to someone with more theme experience than myself. Perhaps @scruffian or @MaggieCabrera? |
|
The issue here comes down to the fact that the Query block doesn't render any markup so alignments don't work well there (exactly like the reusable blocks). I know we've been leaning towards avoiding divs in general but I feel like adding a "div" and layout support to the Query block directly might be a good approach here (just making the Query block itself a container block) |
I do think this makes sense, but I do foresee one issue: currently nested blocks with "Inherit Default Layout" turned on don't play nicely together. So adding that setting to the Query block would make it impossible to get Post Content block content to render the default widths. For example, here's the query block showing a Standard-width Post Title, Wide Featured Image, and Post Content blocks, fully wrapped in a Group block that inherits the default layout:
As you can see, the Post Content block is not showing the correct widths. This happens regardless of whether the Post Content block has "Inherit default layout" turned on or off. That's more or less what would happen today if the Query block had a wrapper and "Inherit default layout" turned on. Today, the way to fix this is to wrap just the title and featured image in a group with "Inherit Default Layout" turned on:
Its not an intuitive fix, so I'd love to see some improvement here either way. |
It doesn't have to "inherit default layout", that's a theme decision:
So it's all theme author's decision, it doesn't prevent anything. |
|
I guess what I'm saying is that while allowing the Query block to inherit default layout will solve these existing patterns, we'll still need to resort to using Group block wrappers inside of the Query block to achieve patterns like that one above that leverage the Post Content block and feature wide/full alignments inside of it. In any case, I guess we should let this PR sit until we have PR to try out a wrapper on the query block? |
|
Oh right, if you want the title to be centered, you need to have a wrapper for it that defines what "centered" actually means and this is regardless of whether it's used inside a Query block or not. |
I agree with @youknowriad and was thinking about it for quite some time. In my mind is not actually a so extraneous I'll start exploring these the next days (hopefully tomorrow) :) |
|
Hey @kjellr - I guess it's okay to close this one as we now have wrappers in |
I'm actually going to re-open this, since the Query Loop layout support doesn't actually solve the issue here. If a layout is enabled on the Query Loop block, that only applies directly to the Post Template Block. Then, the child blocks of that block are all still full-width inside of it. That makes the intended design for the Standard pattern here (a post template that includes both standard and wide alignments) impossible still without the use of an additional Group block:
What we actually need to fix this is either to allow for the Post Template block to have its own "Inherit default layout" control, or to wrap the query patterns in Group blocks like I'd done originally in this PR. (The PR is definitely out of date today, so I'll need to give this a refresh to get it working again.) |
|
The PR's back up to date, and only required the Group block to be added to a single pattern (I'm updating the PR description to better reflect this). This could use a review. Here are the changes: Standard PatternAdds a wide group block wrapper, with layout enabled. This ensures the text uses the content width, while the featured image can be wide.
Image at Left PatternOpts the Query Loop block into inheriting the default layout, and sets the Post Template block to be wide-aligned. This pattern was originally supposed to be wide aligned (hence the
Small Image and Title PatternOpts the Query Loop block into inheriting the default layout, so the rest of the block appears using the content width. This was the original design intent of this one.
|
kjellr
changed the title
|
I'd be happy to add the wrapper here, given how much careful thought has been put into it. I'd appreciate the insights of and others, though, but 👍 👍 |
I haven't done much exploration with Query patterns yet. However, for the few that I have built, I have been adding an extra Group block wrapper for each for the layout. I think this is because the Post Template block doesn't have a layout control of its own. Adding that control seems like the best course of action, avoiding extra containers. |
|
While I understand the reasoning of this PR, it's a bit tricky problem. It might be better for patterns not to use wide/full alignments and ideally they shouldn't be assuming that the theme has a layout defined, or anything. They could use wide/full alignments, if they also add a container that defines the layout explicitly. The reason for this is that alignments depend on their container. So when we set to |
I get what you're saying here, but I think we'll need to figure out a way to fallback to some sort of default if the inherited layout isn't available. We're going to need to be able to use wide/full in patterns like these, and Query Loops in particular get very complicated... I think it's important for users to just choose a pattern with one click and know that it'll match the global layout settings for the rest of their site without having to dig in further.
This is fixable (when a default layout is present at least) by setting a full alignment on the parent containers. I pushed 2006567 to demonstrate. From a technical and usability perspective, I don't think that's a good solution though — it requires setting alignment on multiple layers of blocks and seems quite unreasonable for folks to do. Stepping back for a moment though, I just want these to appear as originally designed. They've been broken for months, and this PR is the closest they've come to looking right again. I'd appreciate some additional help getting this sorted out. |
|
With work rapidly iterating on layout controls and thought, I'm going to close this out as it's very likely out of date at this point. Happy to re-open if others feel strongly. I'd recommend continuing discussion here for general layout confusion/concerns: #36082 |
The currently bundled query block patterns don't work nicely out of the box with the new alignments config as provided in #29335. When inserted into the root of the page, each block is full-width. This is especially noticeable for the "Large" query block pattern.
There are two options here:
This PR opts for #2. Tested in 2021 Blocks, and it seems to work pretty well.