Blocks: Fix the black border appearing too often in blocks

[youknowriad]

Sometimes when a block is selected by clicking on its surroundings (paragraph), a black border appears. This black border is only meant for blocks without any textual content (like image blocks, etc..) when they receive focus and isTyping is true (for example when navigating using arrows).

This PR adds a has-textual-content className for these blocks to avoid seeing this black border when we don't need it.

Nested blocks made this border appear too often.

Testing instructions

• Try clicking and typing in paragraph blocks
• Clicking on the margin/padding of the block
• The block border should not appear.
• It should only appear when navigating using arrows to these non-textual blocks.

[jasmussen]

This seems to work nice. However there appears to be a regression with this rectangle in general, perhaps unrelated to this PR:

Notice that as soon as I insert a placeholder using the slash command, the placeholder itself doesn't have focus. I could swear it used to. I have to press "tab" once to set focus there, whereas previously it had the black rectangle right after inserting. Any thoughts?

Otherwise seems to work great! 👍 👍

[youknowriad]

@jasmussen It should be happening in master too but I fixed it in this PR :)

[jasmussen]

Confirmed! You fixed it! Nice work. Unless someone objects to the code, which I don't, then ship it!

[aduth]

I'm quite concerned that we query select and filter on every change to the block. If it's a matter of avoiding effects that only appear on focusing a specific element (.editor-block-list__block-edit), could we do it on that element's focus event?

[youknowriad]

It could work, but the behavior of the block could change if its content change as well Imagine an image transformed to a text somehow or I don't know, blocks can do what they want.

[aduth]

In the absolute worst case I'd still prefer a MutationObserver monitoring childList and attributes. I expect at least this wouldn't fire the callback on every keypress.

[youknowriad]

Updated using a MutationObserver. Let me know what you think. In my testing, it doesn't trigger on each keypress.

[aduth]

What are the steps to reproduce the original issue? Edit: Ok, I overlooked the steps from the original comment. All the same, couldn't focusTabbable on the block's own focus event handle this? Can you produce a failing case where this would not work per #5140 (comment) ?

I feel like we're going about this wrong. I don't understand why the block would be receiving focus in the first place, and if it's expected that it should, maybe we ought to be transitioning focus into the editable via BlockListBlock's focusTabbable. But I don't think it's expected that it should.

For what it's worth, I was able to reproduce this with nested blocks, but the behavior itself is buggy and not in a way that's remedied by the changes here, but rather symptomatic of a bigger problem.

1. Create Columns block
2. Add some text to one column
3. Press Down
4. First block in column will receive focus / black border

[jasmussen]

Just in case there's doubt. The purpose of this is to indicate what has focus when the block outlines themselves have faded out because we're in isEditing mode.

For example, if you type /image and press Enter, block outlines (the 1px gray border and block controls) are faded out. Instead you should have a black rectangle around the placeholder, indicating that if you tab, you enter the buttons inside the placeholder.

[youknowriad]

This is not only intended to fix the issues with keyboard navigation which could be considered as buggy anyway, but this fixes the black border issue more broadly as it's only intended for non-textual blocks, this makes it clearer with the code.

It's easy to recreate the same issue in master with only just clicks:

• Insert a columns block
• Click on the surroundings of the inner blocks => black border
• Click on the surroundings of the columns block => black border.

We could consider fixing this issue by moving the focus to the inner tabbable each time the block wrapper receives the focus. I don't know if we want this though and also the block wrapper theoritically has tabIndex="0" so I'm wondering if this tabIndex make sense for these textual blocks? Another way to fix this issue is to remove the tabIndex if the block is textual.

[youknowriad]

I'm closing this PR, as no consensus on the approach here. Fine to revisit later.

[jasmussen]

Okay to close this, but then I have to open a separate issue for #5140 (comment), as we need that addressed. But perhaps that's best done separately regardless — it seems like in the past focus was set on the block placeholder when using the slash command to insert, now it's set on the first button inside. I used to think this was desirable behavior, but it falls apart when blocks, and especially placeholder blocks get more complex.

[aduth]

Notice that as soon as I insert a placeholder using the slash command, the placeholder itself doesn't have focus. I could swear it used to. I have to press "tab" once to set focus there, whereas previously it had the black rectangle right after inserting. Any thoughts?

In my testing, after selecting from the autocomplete, the "Upload" button is focused.

This is indeed different than how it used to be, a noted change in behavior resulting from #5102 (specifically remarked at #5102 (comment) ).