Writing Flow: Move selected block focus to BlockListBlock

[aduth]

Alternative to #5092

This pull request seeks to move the behaviors to transition focus to a newly selected block from the WritingFlow component to the BlockListBlock component. In other words, the block is now responsible for focusing itself upon becoming selected. While WritingFlow is meant to abstract / generalize these behaviors, it tended to conflict with nested blocks where we needed to account for a focusable being within an inner block, or DOM reconciliation of external components taking place outside the expected component lifecycle of componentDidUpdate (perhaps a misunderstanding of the flow on my part).

Testing instructions:

Repeat testing instructions from #5092

Verify there are no regressions in writing flow, particularly splitting and merging blocks, and their caret placement. I'm not entirely confident with the changes to not use the initialPosition in considering which tabbable node to use, though it was not entirely clear why it was relevant here.

[youknowriad]

I'm not entirely confident with the changes to not use the initialPosition in considering which tabbable node to use, though it was not entirely clear why it was relevant here.

This is useful in theory if a block have multiple editables and implements merge at the same time, when merging we'll have to focus the last editable and not the first one. But right now, there's no core block with this description.

[youknowriad]

Not sure I understand really the difference between the two implementations. I suspect it's the order of things (lifecycle). We might want to clarify it with a comment to avoid moving back at some point.

[youknowriad]

LGTM 👍

[aduth]

This is useful in theory if a block have multiple editables and implements merge at the same time, when merging we'll have to focus the last editable and not the first one. But right now, there's no core block with this description.

Okay, I think this could be easy enough to implement with a ternary to switch between first and last on the tabbables. Let me implement this real quick.

[aduth]

Noting what's technically a regression here: this.getVisibleTabbables used in the previous implementation used to be more restrictive about the inputs it would focus: namely input and textarea. This is no longer reflected, which has the consequence of other tabbables receiving focus on block selection (e.g. image block "Upload" button).

[jasmussen]

I'm not sure this is a change for the better. The behavior is subtle, but the previous behavior feels more obvious, as it indicates what has focus even in isEditing mode, which becomes very noticable when using the slash command.

Here's the 2.1 behavior:

Here's the 2.3 behavior:

In the old behavior, as soon as you insert a placeholder using the slash command, you see the black focus indicator in isEditing mode, and you can either press delete to undo, or enter to continue writing, perhaps as you're quickly laying out a template. Or you can tab into the contents if you like. This feels like predictable behavior to me.

In the new behavior, your focus is immediately set on the placeholder children, which supposedly could be nice if interacting with the contents of the block was always the next step you wanted to take. But that isn't always the case, sometimes you inserted the wrong thing with the slash command and you can easily delete it. Other times you just want to press Enter and move on.

For example if you insert the separator, nothing has obvious focus and you can't really do anything until you press Tab. In the 2.1 behavior, as soon as you inserted a Separator it had the black focus indicator.

[aduth]

@jasmussen The change in behavior wasn't so much intentional, but it is consistent. In general, I agree with your expectation about quick prototyping and obvious flow of writing. I'd be happy to make refinements here, but we need predictable rules.

When you insert a paragraph block via the autocomplete, where should focus land?
When you insert an image block via the autocomplete, where should focus land?

My assumption based on your explanation is that respectively these should focus the contenteditable and the outline of the image block.

What are the rules that guide these assumptions? Previously, it was trying to find input, textarea, or contentEditable within the block. But what about <input type="button"> ? What if we added a "Upload by URL" input to the image block; should focus still transition there because it now contains a text input?

The previous behavior was inconsistent. And while we might want some inconsistency here, we should be very explicit about when and how those behaviors take place.

[jasmussen]

but we need predictable rules.

Absolutely understand that, and I understand the desire for optimizing code here. Purely talking about the flow. Like I mentioned I previously thought it was desirable to set focus inside the first element of a placeholder block. I even ticketed it here: #3016.

But after discussing, testing, and also looking at #1829, I realized that it would break many flows that are important for the general keyboardability of everything.

When you insert a paragraph block via the autocomplete, where should focus land?
When you insert an image block via the autocomplete, where should focus land?

I would consider the paragraph block an edgecase in this situation. You can easily delete a paragraph block by pressing backspace or delete when inside it. When you insert an image on the other hand, the image itself should be selected, just like if you arrow-key into it, that way you can easily delete it or ⌘X it or perform various operations on it. And if you need to access the contents of the image block, or any other block, you tab into it.

What are the rules that guide these assumptions? Previously, it was trying to find input, textarea, or contentEditable within the block. But what about ? What if we added a "Upload by URL" input to the image block; should focus still transition there because it now contains a text input?

I would consider all the most basic of text blocks — p, ul, ol, h1-6, blockquote — to be somewhat unique, because they give you a caret cursor when you arrow into them.

If you arrow past a block that has no editable text, you get a black rectangle around it — this is in lieu of a text caret, to indicate "this is where your mouse went", and is especially important as when you're keyboarding we fade out all the outlines.

In other words I do think we should have special cases for basic text blocks and every other block, and the key deciding factor is whether using arrow keys to navigate to the block gives you a caret cursor as the first thing you see. If not, then focus shouldn't be set inside.

---

I'm seeing some other regressions, potentially related to these changes but I'm not sure:

• If you press shift tab in a paragraph, you used to get the block toolbar. Now you get a black focus rectangle. Right now the only way you can get the block toolbar using only the keyboard is to select text which unfades the outlines.
• Delete and backspace no longer deletes basic blocks. For example if I arrowkey into an image placeholder and press delete or backspace, that used to delete the block. Same with images.

---

Perhaps it's best with some demos. Here's master:

Here's 2.1:

In 2.1 it's much more keyboardable, and not just because delete and backspace deletes a selected/focused-while-editing block, but Enter also works to create linebreaks.

[aduth]

Some of those issues aren't specific to the changes made in this pull request, but agree they ought to be addressed. I'll plan to spend some time on them today.

[aduth]

If you press shift tab in a paragraph, you used to get the block toolbar. Now you get a black focus rectangle. Right now the only way you can get the block toolbar using only the keyboard is to select text which unfades the outlines.

I'm seeing the same behaviors in master and in v2.1 . It varies though in both cases: When "typing mode" is active (after arrowing or enter some text), pressing Shift-Tab incurs the black border. When not active (and the toolbar is shown), pressing Shift-Tab moves focus into the toolbar.

I suppose it should move into the toolbar in both cases?

[aduth]

I suppose it should move into the toolbar in both cases?

Though is this expected? If I don't see a toolbar, should I expect to be able to tab into one? Or should this be exclusive to when the toolbar is visible?

I have an idea that achieves something similar to #5140 where at least Shift-Tab in a paragraph won't incur the black border focus.

[jasmussen]

I'm seeing the same behaviors in master and in v2.1 . It varies though in both cases: When "typing mode" is active (after arrowing or enter some text), pressing Shift-Tab incurs the black border. When not active (and the toolbar is shown), pressing Shift-Tab moves focus into the toolbar.

Nice catch. Sorry about that, it's hard to follow all the changes.

I suppose it should move into the toolbar in both cases?
Though is this expected? If I don't see a toolbar, should I expect to be able to tab into one? Or should this be exclusive to when the toolbar is visible?

I lean towards the behavior that shift tab should always invoke the toolbar no matter the mode. One of the chief benefits of the block-docked UI has over an on-select popover like Medium has, is that it allows you to press bold first, then start typing, and then press bold to stop writing bold. Whereas with the on-select popup you have to select text first, then make it bold. I don't strongly feel this way, but it seems like it'd be even weirder with Shift Tab moving into the block toolbar only some of the time.

[jasmussen]

In current master there seems to have been either a conscious change to the arrow-key behavior, or a regression. Compare the behavior shown in #5102 (comment), which lets you stay in "isEditing" mode, with what's in master right now:

This could be intentional — exiting isEditing mode when navigating into a non-text-block — but it feels accidental. I don't personally have a strong opinion against this change, just one issue: in the past you could insert a placeholder with the slash command, press tab, space, and pick an image. Now, because the block UI shows, you have to tab through the side UI and block toolbar before you actually enter the block.

Do you know if this was intentional or a regression, somehow, @aduth? CC: @youknowriad @karmatosed

[karmatosed]

I am not aware of an intent here @jasmussen.

[aduth]

It was an intentional change of #5552. Particularly in that it was impossible previously to navigate to toolbar options via keyboard when navigating through to an image block while isTyping.

In master, at this point it would be impossible to use the image toolbar by keyboard alone (without convoluted steps to cancel isTyping by navigating back into the paragraph block)

[jasmussen]

Interesting. Thank you for the context.

I'm also assuming that the item that receives focus when inserted using the slash command is standardized, i.e. we should ideally not poke with it to, for example, set the focus on the placeholder instead? I'm asking because I really miss the ability to:

• type /im, press enter to insert image block
• press "tab tab" to set focus on browse media library

Right now I have to tab through all the side UI and block toolbar first.

If I'm right in the above statement, that we should ideally not set focus manually for cases like these, then perhaps the "Navigation Mode" that @youknowriad is working on, can be an avenue to help improve the tabbing behavior perhaps.

[aduth]

@jasmussen Hm, I might expect that this behavior should be happening (the autofocus on the newly inserted placeholder input), at least based on this logic:

gutenberg/editor/components/block-list/block.js

Lines 106 to 110 in 5a66935

	componentDidMount() {
	if ( this.props.isSelected ) {
	this.focusTabbable();
	}
	}

[jasmussen]

Interesting. Seems worth looking into. How can I help?

[aduth]

Looking closer, focusTabbable is explicitly looking for text inputs. Because the image block does not have any, it's expected that it focuses the wrapper instead.

I'm asking because I really miss the ability to:

Ultimately, the reason it takes so many tabs is because the toolbars and controls are within the block's focusable wrapper, which is the default focus target when there are no text inputs within. There's been some tinkering back and forth on which of these two block wrapper elements are the true focus target:

gutenberg/editor/components/block-list/block.js

Lines 478 to 496 in 424dab7

	<IgnoreNestedEvents
	id={ blockElementId }
	ref={ this.setBlockListRef }
	onMouseOver={ this.maybeHover }
	onMouseOverHandled={ this.hideHoverEffects }
	onMouseLeave={ this.hideHoverEffects }
	className={ wrapperClassName }
	data-type={ block.name }
	onTouchStart={ this.onTouchStart }
	onFocus={ this.onFocus }
	onClick={ this.onClick }
	onKeyDown={ this.deleteOrInsertAfterWrapper }
	tabIndex="0"
	childHandledEvents={ [
	'onDragStart',
	'onMouseDown',
	] }
	{ ...wrapperProps }
	>

gutenberg/editor/components/block-list/block.js

Lines 542 to 549 in 424dab7

	<IgnoreNestedEvents
	ref={ this.bindBlockNode }
	onDragStart={ this.preventDrag }
	onMouseDown={ this.onPointerDown }
	className="editor-block-list__block-edit"
	aria-label={ blockLabel }
	data-block={ uid }
	>

The first is the container where all the controls / toolbars live, and is currently both the tabbable target, and the element on which focus is listening (to be able to select on e.g. movers press). The latter is the immediate wrapper of the block's editable form.

Which is all to say, we might be able to change it to the latter, but off the top of my head I don't recall what cascading effects this might have (and there probably are some).

[jasmussen]

Interesting, thanks so much for the context.

So it's looking for tabbable input fields... Would it make sense to expand that to also look for buttons?

[aduth]

So it's looking for tabbable input fields... Would it make sense to expand that to also look for buttons?

I went digging through the history to determine why this was implemented as it was, and it was the result of feedback from... you 😄 #5102 (comment)

I think the main difference is that it's desirable the user should be able to tab immediately to button options after inserting the block. Is that correct?

[jasmussen]

I went digging through the history to determine why this was implemented as it was, and it was the result of feedback from... you 😄 #5102 (comment)

(ノ°Д°)ノ︵ ┻━┻

☺️

I think the main difference is that it's desirable the user should be able to tab immediately to button options after inserting the block. Is that correct?

To be clear, I'd prefer any solution to be ideally generic, so we don't write some code that only works for some blocks some of the time. That's a meta point.

What I feel would be preferable may not even be feasible, but in my head, the block itself is focus selected as soon as it is inserted. The idea being that right after your insert it, you can press delete, and it's gone. Or you can tab into the contents of the block. Or you can shift tab into the side UI and block toolbar.

Depending on what is best accessibility, your suggestion may be preferable.

[aduth]

Or you can tab into the contents of the block. Or you can shift tab into the side UI and block toolbar.

Yes, this could be done by changing the default focusable wrapper to the inner of the two IgnoreNestedEvents in BlockListBlock. I might want to complement such a change with end-to-end tests verifying the expected behavior.

[aduth]

Created #6687 to track and continue discussion.