New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve moving focus back from the toolbars to the blocks, see `focusSelection()` #5840

Open
afercia opened this Issue Mar 28, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@afercia
Contributor

afercia commented Mar 28, 2018

Noticed while testing the "navigation/edit mode" PR #5709.

The formatting toolbar implements a keyboard-friendly feature: when focus is on the toolbar and the Escape key is pressed, focus is moved back to the selection (or insertion point) in the editable text of the block. See in editor/components/navigable-toolbar/index.js the focusSelection() function.

screen shot 2018-03-28 at 12 42 04

This works pretty well for editable blocks and focus is moved back to the previous selection or caret position.

However, I'd like to propose a few improvements:

1
with the "navigation/edit mode" in #5709 , pressing Escape moves focus back to the selection and then immediately switches back to "Navigation mode" so focus is moved back to the block wrapper.
Seems to me that just stopping the event propagation would fix this.

2
In the inline toolbars (e.g. the ones for the captions):

screen shot 2018-03-28 at 12 42 57

pressing Escape from the inline toolbar does nothing. Also in this case, focus should be moved back to the selection/insertion point in the editable field. I guess focusSelection() should be abstracted a bit and used also for the inline toolbars.

3
Not all blocks have editable fields that can get a selection. For example, consider this scenario:

screen shot 2018-03-28 at 12 50 31

  • working on a paragraph, the selection or insertion point gets stored
  • then move to a block that doesn't have selectable fields, e.g. an image block before setting the image
  • focus the image block toolbar and press Escape
  • focus is moved back to the previous block selection: the paragraph
  • expected: focus should move back to the image block wrapper

In this case, I guess a possible solution would be:

  • check if the current selection pertains to the selected block
  • if not, return early focusSelection() to allow "navigation/edit mode" to move focus back to the block wrapper

/cc @youknowriad any feedback and thoughts welcome

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Mar 28, 2018

The last point might be tricker to achieve but yeah, these all make sense to me 👍

@mtias

This comment has been minimized.

Contributor

mtias commented Apr 2, 2018

Should we consolidate with #3003 ?

@afercia

This comment has been minimized.

Contributor

afercia commented Sep 10, 2018

2 and 3 are still a problem on current master 3.8.0-rc.1 and need to be fixed. 1) actually depends if navigation / edit mode will be implemented, not a great progress there lately and needs to be reconsidered.

@tofumatt

This comment has been minimized.

Member

tofumatt commented Nov 1, 2018

I'm moving this from the merge milestone and into the 5.0 milestone because while I agree it'd be nice to have these improvements, I don't think they're critical. 😄

@afercia

This comment has been minimized.

Contributor

afercia commented Nov 2, 2018

Summarizing the state of the 3 points above:

  1. not so relevant as "navigation/edit mode" in #5709 is stale at the moment
  2. to my understanding, it should be covered by #10699 /Cc @aduth
  3. still a problem and needs to be addressed, as focus is moved back to the wrong block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment