Dismiss inline toolbar by blur

[aduth]

Related: #762

This pull request seeks to explore dismissal of Editable toolbar controls by capturing focus loss instead of relying on events bound to all other elements of a block (or forced dismissal by block becoming otherwise unselected). Notably, some blocks may contain many focusable elements and Editables, and ideally it should not be the responsibility of the block implementer to capture focus or clicks on all elements of the block to ensure that an Editable's toolbars are dismissed. However, this means that they would be responsible for capturing the blur of an editable to unset focus. To me, this seems reasonable to expect.

Implementation notes:

As noted in #762, this one is tough because the specific behavior of clicking on an img while focused on a contenteditable within a wrapper with tabindex (focusable) is rather unexpected: focus changes to the wrapper but the caret still pulses.

http://jsbin.com/kudozoqone/1/edit?html,js,output

The solution here was to add a negative tabIndex to the image to allow it to receive focus but not be accessible by keyboard navigation. This feels a little strange, but (a) with #600 I expect is a temporary solution and (b) the need for blur extends beyond just this image case, so I think it's worth considering this approach anyways.

Testing instructions:

Repeat testing steps from #727

Another case to test which doesn't work in master is that shift-tabbing from caption dismisses inline toolbar.

[aduth]

Hmm, one issue here is that onBlur should really clear the current focus, not set it again to the current block. It works fine for the image block, but when applied to the text block, it causes the user to need to click twice outside a block to unselect it (presumably because focus is being moved back into the block on blur).

[aduth]

Rebased the pull request to resolve conflicts, and to address previous concerns. Specifically, I've added a clearFocus prop passed to the block which dispatches a deselect action when invoked.

[jasmussen]

Took a quick look, and I really dig the code simplicity. Thanks for working on this!

It seems to be a little aggressive in deselecting, though. Here's what I see in master:

Here's what I see in your branch:

Though for whatever reason I had to check out your branch with git checkout origin/update/image-caption-blur, not sure if that would negatively affect this.

[aduth]

Can you clarify what I should be seeing in the GIFs? It lacks context without a cursor.

Not sure about the git thing. Do you have other git remotes than origin?

git remote -v

Might try obliterating the branch and starting over:

git checkout master
git pull
git branch -D update/image-caption-blur
git checkout update/image-caption-blur

[jasmussen]

Can you clarify what I should be seeing in the GIFs? It lacks context without a cursor.

Sorry, I've re-enabled the cursor capturing in my gif recorder.

In the first GIF of master, I click first the caption field to make the inline popup show. Then I click the image, which hides the inline toolbar. This seems desirable to me.

In the GIF image of this branch, I click similarly, first the caption field, then the image itself. But clicking the image not only hides the inline toolbar, but deselects the entire image block.

Do you have other git remotes than origin?

Yes I did in fact, and removing the old one fixed this right up! Thanks so much. 👍 👍

[aduth]

Pushed a rebase, but on final review, I think this could use some more work. Specifically, the behavior of moving from the Caption field to the image by clicking on it causes the entire block to be deselected, when it should merely be the case that the contenteditable focus is removed.

This could tie into #878, where we do a better job of "magically" handling focus on the user's behalf. With #1943, we'll have better mechanisms for communicating between editables and the Editor's global state.

[codecov-io]

Codecov Report

Merging #782 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #782      +/-   ##
==========================================
- Coverage   18.87%   18.86%   -0.01%     
==========================================
  Files         130      130              
  Lines        4207     4209       +2     
  Branches      718      718              
==========================================
  Hits          794      794              
- Misses       2873     2875       +2     
  Partials      540      540

Impacted Files	Coverage Δ
blocks/editable/index.js	0% <0%> (ø)	⬆️
blocks/library/image/index.js	15.15% <0%> (-0.48%)	⬇️
editor/modes/visual-editor/block.js	0% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f6397f...4ac638a. Read the comment docs.