Try refactoring floats

[jasmussen]

It uses the same approach as in this codepen to refactor floats, so they don't have weird margin hacks that fall apart as soon as you stack multiple floats.

It works in this branch for images, and depending on how we like this approach, it should be possible to extend this to all other blocks as well.

Also important to note, the work done here will benefit nested UI as well, and should let us do a negative margin hack to show side UI in nested contexts.

GIF:

On a high level, this works by embracing the max-width that the image block has, and keeping this unchanged. That makes it the same width as the main column. We then float the image inside, instead, allowing the block itself to collapse to a zero height element. This let's text and subsequent blocks wrap around the float as intended.

As far as I can tell, the only weak point in this approach, right now, is that it includes a hack to disable collapsing margins. Here are all the gory details on collapsing margins, but the short of it is this: most themes use collapsing margins. In this PR they are disabled because it allows us to simplify the block boundary code and the fixed toolbar. The benefit to this is that the code is lean for the UI, and just works. The downside is that the image float hack pretty much relies on margins collapsing, and to mitigate this we actually have to add some extra top margin to a floated image, to make sure it aligns with surrounding text. This can become a pain point when themers start to load their style.css file directly into Gutenberg, and suddenly the margins that space their content appear to be doubled, and floated images appearing vertically offset.

But for now, and before I go too far in one direction, this PR currently represents the simplest approach to refactoring floats.

Would love your thoughts.

[youknowriad]

Still seeing some early implementation bugs (jumps in paddings when selecting images, weird white space at the top of wide aligned images) but the fondamental issues seems fixed (floating properly while allowing wide/max-width alignments)

see

To generalize this more easily, I suggested floating the .editor-block-list__block-edit divs instead of floating the content of the block (figure in the case of the image block).

[jasmussen]

Great thoughts, thanks for looking.

To generalize this more easily, I suggested floating the .editor-block-list__block-edit divs instead of floating the content of the block (figure in the case of the image block).

In a way this goes right to the heart of the weak point I mention. To pose that as a question:

How much should we strive to be able to load a vanilla style.css file into the editor? Or do we think there will always be a need to tweak the stylesheet a bit in order for it to work both for the theme and for Gutenberg?

If the latter, then there really isn't any issue with this PR at all, I can blaze ahead and fix the edgecase.

I may have already inadvertently helped answer that question: by floating the figure (or even .editor-block-list__block-edit) instead of the img, we're already deviating from the markup suggested in the linked codepen.

@mtias what are your thoughts on loading the theme stylesheet into the editor? How much customization should you have to do to get that to work?

[jasmussen]

Okay, this is no longer a proof of concept. Per the last few commits, I've addressed all the issues that were present in this branch compared to master. The end result, I feel, is much stronger than what's in master right now.

It's not as wide-ranging a refactor as I had perhaps hoped we could make — it seems themes we have to embrace that there will have to be a few editor specific styles even if we load the theme stylesheet itself — but this is no different from what was in master, and could potentially be mitigated/looked at separately.

I also restored the "data-resized" hack even for images.

There are also still some issues with the side/hover UI stacking — but perhaps we should simply show a nested UI when hovering floats.

However all of those issues are also present in master, the big difference with this refactor is that it makes floats behave in the editor, like they do on the front-end, and it drastically simplifies the floats CSS code and makes it more elegant and understandable.

As such, I think this might actually be ready for its first review, with the serious consideration of merging this in mind.

GIF:

[youknowriad]

Can you explain why the embed block needs specific handling of floats? Trying to understand to see if we can come up with something generic to avoid all styles in individual blocks

[youknowriad]

I forked this branch and added this commit 6f5a765 to avoid any special casing in blocks. Why can't this work?

[youknowriad]

There are still some specific styles for the button block because of the URL input but that's fine. So now the generic use-case when aligning left/right assigns a max-width unless the data-resized is true which indicates the width should be automatic because of the content of the block.

What do you think?

[jasmussen]

I like that. Sounds good to me for sure. I'll see if I can port your changes, otherwise feel free to push to the branch.

[youknowriad]

So I'm seeing two small issues:

The toolbar is misaligned by 1px (see the bottom border)

Also, the toolbar doesn't seem to follow the block when right aligned.

[jasmussen]

I'll see if I can fix the misalignment — gonna try and find out what the source is. I was sure I'd caught that.

Also, the toolbar doesn't seem to follow the block when right aligned.

I'm not completely convinced there's a clean way to address that as part of this setup. I was going to look into this separately once this branch was merged in, in hopes that many of the float related issues that @gziolo reported in the past were addressed by the better float system. As part of that, and as part of the need for tweaks to nested UI as well, I was hoping to find a solution that worked for both.

[youknowriad]

I'm not completely convinced there's a clean way to address that as part of this setup. I was going to look into this separately once this branch was merged in,

Fine for me, I don't think it's critical

[youknowriad]

@jasmussen So, I pushed my commit in this branch 👍

[jasmussen]

Tried right aligning the toolbar when an object is right floated:

Seems to work pretty well no?

Note — stil side-UI changes to do. As mentioned, I'm thinking of a UI that works for mobile, nested blocks and floated blocks, I think that might be the solution.

[youknowriad]

This is great 👍 from me.

[jasmussen]

Here's what I'd like to do with the side-UI for floated items:

Essentially, as soon as an item gets floated, it gets the mobile UI, and no side inserters. This solves the stacking issue, and keeps the context too.

I can do that as part of this branch, but I'd like to do it separately just because this is a lot of red code.

[mtias]

How much customization should you have to do to get that to work?

Ideally none. If we have themes doing structural updates it's going to be virtually impossible to ensure consistency in UX.

[jasmussen]

Ideally none. If we have themes doing structural updates it's going to be virtually impossible to ensure consistency in UX.

Thanks for looking. This is a noble goal, one I happen to agree with. Since working on this particular refactor, I think that goal is a ways away, but I do think this PR actually gets us much closer than where master is right now. I'll try and summarize the problems solved, and the challenges remaining.

Problems this PR solves:

• Makes floats vanilla, and makes it much more predictable how themes can float items
• Has a simplified main column width variable, allowing us to trivially fix Block width (content width) needs to be resizeable #1483
• Simplifies the block toolbar code slightly
• Takes steps towards a block UI that works in nested contexts

Problems we'll need to tackle:

• The block markup is different front and back, so the item floated, if it should map between them, will require some noodling. In this PR we float the .editor-block-list__block-edit inside the floated block, which in the codepen suggested for themes would map to the img inside an image block. The question here is — which direction do we need to map the floats? From the theme into Gutenberg, or is it okay that Gutenberg comes with its own styles here?
• Related to the above — a WordPress theme can use vw units and opt into fullwide images. However this will not work very well in the admin context, as this unit won't take into account left and right navigation and meta sidebars. As such in the same vein, perhaps Gutenberg should be opinionated about how both floats and wide blocks are handled?
• Block side paddings — we can offset this with negative margins, but if a theme does something with paddings naturally as part of the theme, this might interfere if the stylesheet was loaded into the editor.
• Collapsing margins. If a p has a margin: 2em 0, three adjecent paragraphs will have collapsing margins when rendered in a vanilla environment, effectively resulting in 2em margin between those paragraphs, and not a 4em margin as you might imagine. This would be fine, but when we paint a 1px border around a selected paragraph, we include the margin, collapsed or not, and we include this margin when positioning the sticky toolbar as well. A bit more on this in a second.

Problems I doubt we can solve without an iframe:

• Fixed or Sticky elements in a theme — a position: relative; on the parent, in this case the gutenberg editor container, would be insufficient in constraining the element to be inside.

Next steps

Given that this PR makes steps forward compared to what's in master, I feel like it's still worth merging and then looking at ways to iterate on. The PR may not address all issues right now, but it addresses two big problems with the previous implementation: theme width, and floats.

Perhaps part of the solution is to leverage the existing split we have between a block style.scss and a block editor.scss, and simply try and put the burden on Gutenberg, not the theme, to ensure things look and work 1:1. The biggest problem here is probably still the collapsing margins. The way we position the sticky block toolbar right now, it relies on the box model, and therefore block margin, to position itself, which means it will be differently positioned on a block where margins have collapsed, compared to a block where margins are not collapsed. This could potentially be solved with some good ideas, perhaps a little translate thrown into the mix, but it's definitely a headache we'll run into.

To be clear, this PR, and the approach to floating, does not rely on hacks that make it any harder than it already was in master to

[mtias]

This is a problem in master as well, but looks like "wide" and "full" should clear floats (the two image gallery):

[jasmussen]

Pushed a fix to clear floats when wide or fullwide. This introduces a z index issue, because floats are at a higher z index:

But this is something we want to look at regardless once this is in. Or should a fix be part of this PR?

[youknowriad]

Pushed a fix to clear floats when wide or fullwide. This introduces a z index issue, because floats are at a higher z index:

This also means we could use this same z-index for wide/full aligned blocks because they can't conflict with floats anymore since they clear them. could be an easy fix.

[jasmussen]

You're right!

[jasmussen]

Fixing collapsing borders is probably going to have to be the next step in a refactor after this PR. The columns block is a good example of why. Right now the columns block itself has margin before and after. Each column inside the columns block has margin before and after. If those margins were allowed to collapse, the result would be a much more natural flow, where the columns container margins would collapse.

Here is a codepen that shows a proof of concept for how we can do that, but I'd probably need to pair with someone to do this, as it would require a refactor of how the contextual block toolbar is inserted. https://codepen.io/joen/pen/aqKLqa?editors=1100

[jasmussen]

Got a 👍 👍 in a chat. Going to merge this in as it makes strides in improving how floats behave. It doesn't go all the way, but it takes big necessary steps.