Keep the focus in the block navigation when user's last interaction was with the block navigation #22524

adamziel · 2020-05-21T14:24:11Z

Description

PR #22427 introduced an experimental ellipsis menu to the block navigation on the experimental nav management screen. Unfortunately, duplicating a block using that menu transfers the focus back to the editor area. To provide the best possible experience for users using mostly their keyboards, we should do the following:

When the block is duplicated using the ellipsis menu in block navigation, we should .focus() the new item in the block navigation.
When the block is duplicated using the editor area menu, we should .focus() the new block in the editor area.

I've been thinking about possible ways of approaching this issue and concluded that the editor must know whether or not it is allowed to grab the focus. This PR introduces the new editor state variable called isAutoFocusEnabled. The editor will only focus() on the new block if isAutoFocusEnabled === true.

How has this been tested?

Go to the experimental navigation page
Add a few menu items
Duplicate a menu item using the controls in the editor area, confirm the new block in the editor area gained focus.
Duplicate a menu item using the controls in the block navigation area, confirm the new block navigation item gained focus.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

…as with the block navigation

adamziel · 2020-05-21T14:25:38Z

One last issue here is that the block is not possible to select using the block navigation. This is an issue on the master branch too though, so we could probably address it in another PR.

github-actions · 2020-05-21T14:29:50Z

Size Change: +619 B (0%)

Total Size: 1.12 MB

Filename	Size	Change
`build/block-directory/index.js`	6.93 kB	+2 B (0%)
`build/block-editor/index.js`	105 kB	+234 B (0%)
`build/block-library/index.js`	119 kB	+1 B
`build/components/index.js`	190 kB	+21 B (0%)
`build/edit-navigation/index.js`	6.99 kB	+359 B (5%)	🔍
`build/edit-post/index.js`	302 kB	+4 B (0%)
`build/edit-widgets/index.js`	7.73 kB	+1 B
`build/editor/index.js`	44.6 kB	+1 B
`build/primitives/index.js`	1.5 kB	-1 B
`build/server-side-render/index.js`	2.67 kB	-3 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.02 kB	0 B
`build/annotations/index.js`	3.62 kB	0 B
`build/api-fetch/index.js`	3.39 kB	0 B
`build/autop/index.js`	2.83 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/style-rtl.css`	790 B	0 B
`build/block-directory/style.css`	791 B	0 B
`build/block-editor/style-rtl.css`	10.9 kB	0 B
`build/block-editor/style.css`	10.9 kB	0 B
`build/block-library/editor-rtl.css`	7.17 kB	0 B
`build/block-library/editor.css`	7.17 kB	0 B
`build/block-library/style-rtl.css`	7.48 kB	0 B
`build/block-library/style.css`	7.48 kB	0 B
`build/block-library/theme-rtl.css`	684 B	0 B
`build/block-library/theme.css`	686 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	48.1 kB	0 B
`build/components/style-rtl.css`	17.1 kB	0 B
`build/components/style.css`	17.1 kB	0 B
`build/compose/index.js`	9.24 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/data/index.js`	8.42 kB	0 B
`build/date/index.js`	5.47 kB	0 B
`build/deprecated/index.js`	771 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.11 kB	0 B
`build/edit-navigation/style-rtl.css`	857 B	0 B
`build/edit-navigation/style.css`	856 B	0 B
`build/edit-post/style-rtl.css`	12.2 kB	0 B
`build/edit-post/style.css`	12.2 kB	0 B
`build/edit-site/index.js`	12.9 kB	0 B
`build/edit-site/style-rtl.css`	5.31 kB	0 B
`build/edit-site/style.css`	5.31 kB	0 B
`build/edit-widgets/style-rtl.css`	4.59 kB	0 B
`build/edit-widgets/style.css`	4.59 kB	0 B
`build/editor/editor-styles-rtl.css`	425 B	0 B
`build/editor/editor-styles.css`	428 B	0 B
`build/editor/style-rtl.css`	5.06 kB	0 B
`build/editor/style.css`	5.06 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.64 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	711 B	0 B
`build/keyboard-shortcuts/index.js`	2.51 kB	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/index.js`	3.13 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	5.29 kB	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/index.js`	2.56 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/redux-routine/index.js`	2.85 kB	0 B
`build/rich-text/index.js`	14.8 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.02 kB	0 B
`build/viewport/index.js`	1.84 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.17 kB	0 B

_{compressed-size-action}

youknowriad

I know that in other similar use-cases ( replaceBlocks action), instead of having an auto-focus mode, we rely on a flag in the action to not update the selection. can't tell which approach is better but seems like some consistency would be good?

youknowriad · 2020-05-21T15:01:20Z

I've been thinking about possible ways of approaching this issue and concluded that the editor must know whether or not it is allowed to grab the focus. This PR introduces the new editor state variable called isAutoFocusEnabled.

It's not clear to me what this mode is for since in the same dropdown if you click a block, the idea is to go focus the block which kind of defeats the purpose of the mode. Is it an indication that having more descriptive actions is maybe a better approach?

draganescu · 2020-05-21T15:08:46Z

The navigator should be a summarized view of the block hierarchy. They way things are going, considering we plan to manipulate all the document's structure from the navigator, we need both things to work, that is transferring the selection to the editor and keeping the selection in the navigator.

I would explicitly add an action, "Go to block", that selects the block from the editor, instead of having a mode that auto selects the block when I click on it in the navigator.

The focus problem is that when I work in the context of the summarized view of the block hierarchy I do not want to be transferred to the editor unexpectedly. I do wonder if the editor needs anything changed, or if the navigator itself has to manage its focus more thoroughly.

adamziel · 2020-05-21T15:20:37Z

@youknowriad @draganescu The culprit is the following excerpt from the block-wrapper.js:

gutenberg/packages/block-editor/src/components/block-list/block-wrapper.js

Lines 126 to 130 in e8b9de4

    
           useEffect( () => { 
        
           	if ( ! isMultiSelecting && ! isNavigationMode && isSelected ) { 
        
           		focusTabbable(); 
        
           	} 
        
           }, [ isSelected, isMultiSelecting, isNavigationMode ] );

As you can see, it will "steal" the focus when the block becomes selected.

In order to move along without an autofocus mode, we would either need to:

Make sure the block is NOT selected when the user interacts with/duplicates a related block navigation item, which may or may not defeat it's purpose, or...
Figure out a condition different than isAutoFocusEnabled that would the prevent block-wrapper from grabbing the focus.

adamziel · 2020-05-21T15:24:17Z

I would explicitly add an action, "Go to block", that selects the block from the editor, instead of having a mode that auto selects the block when I click on it in the navigator.

@draganescu it's the other way around :-) that mode prevents the editor from performing it's default action (selecting the block)

youknowriad · 2020-05-21T15:24:37Z

As you can see, it will "steal" the focus when the block becomes selected.

Yes, I know but unless I'm wrong it could be solved by saying "duplicate but don't select"?

adamziel · 2020-05-21T15:25:51Z

Yes, I know but unless I'm wrong it could be solved by saying "duplicate but don't select"?

Right, that's correct. My assumption was that we want to select after duplicating though - that's what happens when you duplicate from the editor. I wonder, maybe it's time to take a step back and consider separating the selection in the navigator from the selection in the editor? At least on that experimental screen.

youknowriad · 2020-05-21T15:27:04Z

Maybe it's time to take a step back and consider separating the selection in the navigator from the selection in the editor?

Does the selection in the navigation serve other purposes than just an "indication" about the block selected block in the editor?

adamziel · 2020-05-21T15:29:54Z

Does the selection in the navigation serve other purposes than just an "indication" about the block selected int the editor?

It's not obvious to me at the moment. In the post editor, one of the purposes the navigator serves is to make it easy to reach deeply nested blocks. You click on the navigator item and the block becomes selected. In the experimental navigation screen it is different. Even though we use the same navigator component, we customized it so that it no longer selects a related block on click - instead it makes it possible to quickly edit the link text.

Let's put this PR on hold for now - I'll start a discussion in the original issue #22089

adamziel · 2020-05-27T12:36:56Z

It seems like we're moving away from navigator items being editable by default which also solves the underlying issue behind this PR. It may re-emerge at some point, but for now this PR may be safely closed.

Keep the focus in the block navigation when user's last interaction w…

4512623

…as with the block navigation

adamziel added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Navigation Affects the Navigation Block labels May 21, 2020

adamziel requested review from draganescu, noisysocks and talldan May 21, 2020 14:24

adamziel requested review from ajitbohra, chrisvanpatten, ellatrix, jorgefilipecosta, mkaz and youknowriad as code owners May 21, 2020 14:24

adamziel self-assigned this May 21, 2020

Update MenuEditorBody

efefa52

youknowriad reviewed May 21, 2020

View reviewed changes

adamziel mentioned this pull request May 21, 2020

Navigator iterations: ellipsis and movers #22089

Closed

adamziel closed this May 27, 2020

youknowriad deleted the fix/ellipsis-menu-focus branch May 27, 2020 22:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep the focus in the block navigation when user's last interaction was with the block navigation #22524

Keep the focus in the block navigation when user's last interaction was with the block navigation #22524

adamziel commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020

github-actions bot commented May 21, 2020 •

edited

Loading

youknowriad left a comment

youknowriad commented May 21, 2020 •

edited

Loading

draganescu commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading

youknowriad commented May 21, 2020

adamziel commented May 21, 2020 •

edited

Loading

youknowriad commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading

adamziel commented May 27, 2020

Keep the focus in the block navigation when user's last interaction was with the block navigation #22524

Keep the focus in the block navigation when user's last interaction was with the block navigation #22524

Conversation

adamziel commented May 21, 2020 • edited Loading

Description

How has this been tested?

Types of changes

Checklist:

adamziel commented May 21, 2020

github-actions bot commented May 21, 2020 • edited Loading

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad commented May 21, 2020 • edited Loading

draganescu commented May 21, 2020 • edited Loading

adamziel commented May 21, 2020 • edited Loading

adamziel commented May 21, 2020 • edited Loading

youknowriad commented May 21, 2020

adamziel commented May 21, 2020 • edited Loading

youknowriad commented May 21, 2020 • edited Loading

adamziel commented May 21, 2020 • edited Loading

adamziel commented May 27, 2020

adamziel commented May 21, 2020 •

edited

Loading

github-actions bot commented May 21, 2020 •

edited

Loading

youknowriad commented May 21, 2020 •

edited

Loading

draganescu commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading

youknowriad commented May 21, 2020 •

edited

Loading

adamziel commented May 21, 2020 •

edited

Loading