Block List: Allow clicking behind block next to toolbar

[aduth]

Edit: This pull request has been updated to not include any changes to clickable area for a block, since it's expected this would conflict with upcoming changes to introduce drag-and-drop (#4115). Instead, this only includes the changes noted for toolbar effective width:

Further, improvements have been made to not consider the area to the right of toolbar actions as part of the toolbar, so it is possible to select a preceding block even when a contextual toolbar is visible.

---

Original Text:

This pull request seeks to improve block deselection behavior when clicking "outside" of a block. The issue is that because the block container encompasses more than just the visible editable area of a block, while the user might often think they're clicking outside the block, they're merely clicking on its container.

Visualization:

Clicking within the green highlighted regions would previously not deselect a block.

The solution proposed here is one I've been reluctant to introduce, largely because I believed * CSS selectors were non-performant (related). Based on some relevant readings ([1], [2], [3]), I'm led to believe these concerns are overstated and, all the same, the user impact cost/benefit is more largely in favor of optimizing selection behaviors.

Further, improvements have been made to not consider the area to the right of toolbar actions as part of the toolbar, so it is possible to select a preceding block even when a contextual toolbar is visible.

Testing instructions:

Verify that it's easy to deselect a block, and that there are no regressions in using controls of a block.

[aduth]

Alternatively, we could try to reduce the amount of padding to the left and right of the block. I had assumed this was necessary to provide some breathing room for blocks as the viewport width decreases, but I'm starting to second-guess myself on this assumption. @jasmussen can you recall if and why we need this extra padding?

[jasmussen]

This padding is necessary for the side UI to work on hover. It will also be used as the draggable space when drag and drop gets merged in.

[aduth]

Aha, so in that case:

• Hovering a block and trying to click its "More Options" menu has regressed with these changes
• If drag-and-drop is meant to use this space as its draggable start-point, do we then want this void zone to continue to exist (I believe "yes")? At least with drag-and-drop, assuming we're using cursor: grab, it would be easier to tell when clicking would cause a deselection than it would currently.

Neither of the above seem to depend on the padding being so much wider than the block, just enough that moving the cursor between the block editable area and the "More Options" or movers buttons doesn't hide the hover UI.

I'm a bit curious to see how this plays out with drag-and-drop though. The cursor changes alone might help the situation significantly, because it's otherwise caused me a fair bit of frustration trying to deselect blocks, not knowing whether a click would be effective in achieving this goal.

In any case, the contextual toolbar style changes still seem worth considering.

[jasmussen]

The purpose of this PR is still very much in need, and yes, the hope is that the cursor for drag and drop will be sufficient indicator. That sort of remains to be tested, and there are some mockups that feature drag dots to alleviate if it doesn't.

[aduth]

After taking #4115 for a spin, I can confirm it feels much better to have the cursor property changes as an indicator of whether deselection would be expected to occur on a click.

Which makes me think that the pointer-events changes aren't needed, as they'd conflict both with hover UI and drag-and-drop. Do you disagree?

[jasmussen]

Without having had time for a deep dive, no, I do not disagree.

[aduth]

Rebased to drop the pointer-events style, and updated the original comment accordingly.

[aduth]

For future reference, there is a CSS style width: max-content that can serve in place of display: table, but it has poor browser support:

https://caniuse.com/#feat=intrinsic-width

[aduth]

I found that, as inline-flex, Firefox would add some height when rendered within the display: table wrapper. I see no reason why it needs to be inline-flex, as it's only ever rendered as the sole child in all of its usage.

[jasmussen]

Nice work.

The experience of using this seems much improved compared to before. I could swear there was even a ticket for this, that this PR will close, but I couldn't find it.

However a few things — I'm seeing some issues around floats. This is potentially as a result of a bad rebase as of the recent float refactor I merged in, in any case I'm not seeing it in master.

First off is a z index issue with the toolbar:

The floats refactor contained a specific fix for that, to elevate the toolbar to the same level as a floated image.

Second is a 1px shift for the UI controls when selecting a floated image:

That's also something that I had to deal with in the floats refactor, and it's not in master.

Finally, it would still be nice if you could deselect a block by clicking below it:

But that should probably be tackled separately, if at all. I understand that there are challenges with this, and the fact that you might have metaboxes down there.

[aduth]

@jasmussen I've rebased the branch thinking that it might have been out of date with your float refactoring. I'm not able to reproduce the issues you mention. Could you give another look and let me know if it's working as expected for out?

The benefit of these changes isn't as much because the area immediately to the right of the toolbar is now the sibling inserter, but it still feels like a change worth making.

[aduth]

Alternatively, if we're afraid of consequences of using display: table, we could use the instrinsic max-content, acknowledging the fact that it has poor browser support , but treating is more as a progressive enhancement.

[jasmussen]

This seems to be working better than before insofar as I'm not seeing the toolbar z index issue.

But I'm still seeing the 1px upwards shift when you select and deselect a block:

I'm not seeing that in master. I bet it's the removal of this line: https://github.com/WordPress/gutenberg/pull/5180/files#diff-00ebf096f7c483426f2a78d70d9cbd22L491

[aduth]

Who would have thought such few lines of code would require so many iterations 😄

Updated to remove the unnecessary margin.

Now I'm noticing issues with toolbar placement for floated image, because of display: table. I was inclined to say "screw it, let's do #5180 (comment)", but in fact it's less to do with table display, and more in allowing the width to collapse. width: min-content has the same problem. Will require further troubleshooting...

[jasmussen]

Is there any testing I can do to help here?

[aduth]

@jasmussen I've not revisited this one. If you have spare cycles to evaluate the placement of the floated toolbar, feel free. Otherwise I can plan to take a look, most likely tomorrow at the earliest.

[jasmussen]

Took a look, and I do understand the problem now. When editor-block-contextual-toolbar is table, the width of the toolbar wraps around the child element. But when that's abs positioned right, it breaks.

I can't think of any easy solutions right now. But I do suspect that we will revisit the toolbar on floated images, perhaps as part of the nesting UI improvements. Maybe the issue can be revisited then?

In other words, maybe don't apply table on floated images for now?

[ZebulanStanphill]

Is this PR still relevant?

[aduth]

@ZebulanStanphill I would have thought it was, since I was not aware of any changes which would have affected the behavior here, though in testing it just now the issue appears to no longer be present. When hovering the cursor to the right of a a block's toolbar, the hover effects of the preceding block are activated as I'd expect.

[ZebulanStanphill]

Related: #8880, #8881, and #8883.