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

Limit content width for things aligned left/right #3504

Merged
merged 4 commits into from
Mar 24, 2021

Conversation

pbking
Copy link
Contributor

@pbking pbking commented Mar 22, 2021

This change limits the size to a configurable width (set to 50%) for any elements that are aligned left/right.

Additionally a "float reset" is added as an :after to the group block in order to get content that follows the block to stay out of it.

There are still differences between the view and editor that are the responsibility of Gutenberg; Content doesn't appear to follow the "normal width" constraints of the view.

This leverages primarily the demo content found here: https://theamdemo.wordpress.com/2019/06/25/gutenberg-image/
But in order to achieve the design the local content I used put all 3 image blocks demoing alignment into a single group that was constrained to "normal" width. Additionally the "float reset" applied to the group block for the view (noted above) doesn't work for the editor.

View:
image

@jffng
Copy link
Contributor

jffng commented Mar 22, 2021

I'm having trouble testing this because left/right alignments are not showing up for me on image blocks:

Screen Shot 2021-03-22 at 2 18 08 PM

How can I enable them using the new alignments system?

Additionally a "float reset" is added as an :after to the group block in order to get content that follows the block to stay out of it.

Granted I couldn't test it, I'm not sure this is the behavior we should create. I think my expectation is that if I align something to the left / right, I want the rest of content to be placed in-line with it and wrap around it, for example if I'm placing an image in line with some text:

Screen Shot 2021-03-22 at 2 22 36 PM

@pbking
Copy link
Contributor Author

pbking commented Mar 22, 2021

To achieve that I understand that all of that content would need to be put into a group that has "normal" alignment. Then content in there can be aligned left/right. Content can EITHER be normal/wide/full OR left/right. If the container has dictated "use inherited layout" or assigned normal/wide values then the elements in that container cannot be left/right aligned. Since (in this theme) the content area as "inherit layout" set (in the templates) then any top-level content is limited to normal/wide/full.

pbking and others added 2 commits March 22, 2021 14:31
fixed spellilng

Co-authored-by: Jeff Ong <jeff.ong@automattic.com>
fixed spellilng

Co-authored-by: Jeff Ong <jeff.ong@automattic.com>
@jffng
Copy link
Contributor

jffng commented Mar 22, 2021

I understand now, thanks for the explanation!

I also now understand the need for a "clear" on the group block. I agree it should be added by Gutenberg, especially since this fix doesn't yet work for the editor because the ::after pseudoelement is used by the block-list editor component and is absolutely positioned: https://github.com/WordPress/gutenberg/blob/50cdfbeae59cf0fe91faa2183877b3b7fc60dd3e/packages/block-editor/src/components/block-list/style.scss#L263

@danieldudzic
Copy link
Contributor

Any ideas on applying the clear to the Group block inside the editor, and not breaking the block-list component?

@scruffian
Copy link
Member

There's a Gutenberg issue for this here: WordPress/gutenberg#10299

@scruffian
Copy link
Member

I'm not sure why we need a clear on the Group block, but I think that should live in Gutenberg.

The alignment styles look ok for now, we just need to update the mixin.

I think the whole left/right alignments thing is going to be a problem going forward but that's a topic for a different discussion.

@pbking
Copy link
Contributor Author

pbking commented Mar 23, 2021

The clear is to account for this behavior:
(without the clear fix)
image

(with the clear fix)
image

@scruffian
Copy link
Member

Right, but if I wanted that behaviour I could also use a columns block so I'm not sure what a user would expect...

@pbking
Copy link
Contributor Author

pbking commented Mar 23, 2021

I believe a user would expect that things that aren't in a group (those that come after) to ... not be in the group. It's especially obvious if you set a background color for the group, then the content outside of the group starts in the group, doesn't change the size of the group and flows out:
image

(with the clear fix: the behavior I would expect from a group block)
image

@pbking
Copy link
Contributor Author

pbking commented Mar 23, 2021

This was also discussed here: WordPress/gutenberg#4127

Regarding the float clearfixes I'm not sure why any container block that allows for left/right alignment wouldn't benefit from a float clearing of the type added as a clearfix in this PR.

That said it's an old problem, however something that Gutenberg will need to fix, and hopefully will as the left/right alignment stuff continues to settle. (I agree with the above; that it's likely to be messy for a bit longer).

To that end (since this is a polyfill theme after all) I think we should ship the group clearfix, but as it stands it's incomplete and I'm not sure how to make it work in the editor. Even without it though the editor isn't quite spot on regarding that anyway, so perhaps it just a thing to not worry about right now.

@jffng
Copy link
Contributor

jffng commented Mar 23, 2021

I agree with @pbking and updated the clearfix to address the editor:

Screen Shot 2021-03-23 at 2 14 15 PM

@pbking
Copy link
Contributor Author

pbking commented Mar 24, 2021

👍 That last change brings the behavior in view and editor into alignment with my understanding of expectations. Bringing this in.

@pbking pbking closed this Mar 24, 2021
@pbking pbking reopened this Mar 24, 2021
@pbking pbking merged commit 4c23164 into make/blank-canvas-blocks Mar 24, 2021
@pbking pbking deleted the add/bcb-lr-alignment-constraints branch March 25, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants