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

Post excerpt: Add layout and align / wide support #45004

Closed
wants to merge 1 commit into from

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Oct 17, 2022

What?

Adds align wide, full width, and layout support to the post excerpt block.
Closes #35697

Why?

The block was missing these layout features, and as an example, a post content block and a post excerpt block would not align with each other (since the post content block default is to enable "Inner blocks use content width").

How?

Adds align wide, full, and experimental layout block support.

Testing Instructions

Please test the post excerpt block in both the block editor and site editor.
Confirm that the wide width, full width and layout settings (including justifications) work correctly, and display correctly on the front.

Note that depending on where you place the block, you may need to place it inside a group with the "inner blocks use content width" setting enabled.

Optionally, in the Site Editor, open or create the template for single posts. Add a post excerpt block and a post content block,
and compare the position of the content inside these blocks, with different settings enabled. It should be possible to align the block so that the text position is the same in both blocks.

Screenshots or screencast

Before:
Post excerpt is missing layout settings and is full width

After:
post excerpt has layout settings enabled and the text aligns with the post content block that is above it.

@carolinan
Copy link
Contributor Author

carolinan commented Oct 17, 2022

Pinging @aaronrobertshaw since this PR suggest that layout support is added to the excerpt block. I am not sure if this is suitable. In a sense, the block is a container for other blocks, but the inner blocks themselves are not selectable unless you are adding a custom excerpt text.
I am not sure how to make the post content block and the post excerpt block align consistently if we can not add the layout support 🤔

@carolinan carolinan marked this pull request as ready for review October 17, 2022 07:46
@carolinan carolinan added [Block] Post Excerpt Affects the Post Excerpt Block [Type] Enhancement A suggestion for improvement. labels Oct 17, 2022
@aaronrobertshaw
Copy link
Contributor

Thanks for the ping @carolinan.

I am not sure if this is suitable. In a sense, the block is a container for other blocks, but the inner blocks themselves are not selectable unless you are adding a custom excerpt text.

This is a good question. @andrewserong and @tellthemachines have been working a lot on layout support and related issues lately, they might be better positioned to evaluate the right course of action for this block.

In the current tracking issue for layout support adoption, there was a brief outline regarding eligibility for blocks wishing to add layout support. The Post Excerpt block didn't make the list of candidates there.

To be eligible for the Layout support, blocks must be containers for other blocks. For example, if the block uses useInnerBlocksProps then it is a likely candidate for benefiting from the layout support.

I'm not sure how rigid those requirements are, but if we are looking to bend them, I'd like to get the thoughts of those working on layout support at a deeper level early.

@andrewserong
Copy link
Contributor

Thanks for the ping!

I'm not sure how rigid those requirements are, but if we are looking to bend them, I'd like to get the thoughts of those working on layout support at a deeper level early.

Yes, my gut feeling is that the layout support opt ins should be restricted to formal container blocks for now (that is, blocks that are containers for other editable blocks and use the innerBlocksProps for defining inner wrappers, etc)

Since we're still trying to get the layout support to work correctly for the existing container blocks, I think it might be premature to attempt to use it for blocks outside of that scope.

@carolinan carolinan added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Oct 18, 2022
@tellthemachines
Copy link
Contributor

I'm inclined to say we shouldn't add Layout here because Post Excerpt never has inner blocks (unlike Post Content, that contains all the blocks in the post) and Layout should only be applied to containers for other blocks. This is because Layout only affects the children of a block, not the block itself.

This is an interesting problem though, because Post Excerpt is a case of a block that doesn't have inner blocks but does have inner HTML elements, unlike, say, Paragraph that only has a single element with text content. I think it's meant to behave like Quote or List, that also have inner elements (Quote even has inner blocks now) but behave like a single inline block.

The fact that the block has inner elements is what allows Layout to kind of work here. Should we add Layout to this kind of blocks? The advantage of doing so is being able to give them content width; however, they won't ever support wide or full width children, because their children aren't configurable as blocks.

Ultimately though, we can only enforce what we see as best practices in Core, and I'm sure once the API is out in the wild we'll be seeing all sorts of unorthodox usage 😄

One thing to bear in mind is we're changing how layout classnames are applied to blocks in #44600. When that goes through, adding layout support to a block without inner blocks, or even a block that doesn't use the InnerBlocks component, will require adding in the classnames manually in the edit function in addition to the config in block.json.

@carolinan
Copy link
Contributor Author

These reasons makes sense to me. On the other hand I do think it is common to switch the post content block and the post excerpt block in templates, and I think they need to align better so that the design is not drastically different.
The issue is less visible if the query/post template uses columns to position the content, and on small widths.

@tellthemachines
Copy link
Contributor

The issue is less visible if the query/post template uses columns to position the content, and on small widths

When Post Excerpt is inside Query, you can just set Layout in the Query block. I think this would only be an issue when using Post Excerpt outside of a Query, though I'm not sure if there's a ton of use cases for that.

@carolinan
Copy link
Contributor Author

Closing as 'wont fix'

@carolinan carolinan closed this Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Excerpt Affects the Post Excerpt Block [Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post excerpt: bring in more controls
4 participants