Focus the currently focused node when it mounts

[pcardune]

Fixes #591

This ended up being surprisingly complicated. The solution to the actual problem was simple: add a useEffect() that focuses the currently focused node when it mounts. Doing this worked fine and all the tests passed, except there was a cryptic react warning message that showed up when the tests ran. The warning only showed up in tests and not when doing exactly the same thing in the browser.

The issue has to do with the timing of calls to focus(), though not the call to focus() that I added, but rather one that's been in there already for a while, and which is deeply buried in codemirror in the call to setBookmark(). I don't know why the warnings didn't show up until I made this change. In any case, you are theoretically not supposed to monkey with the dom (e.g. by calling focus()) while react is rendering, and instead use useEffect() to wait until rendering is finished, but we were calling setBookmark() while rendering ToplevelBlockEditable.

Anyway, to fix the issue, I moved the call to setBookmark() to happen inside a useEffect(), which resulted in a different problem. The order in which useEffect() callbacks are called is depth first. So the useEffect() inside NodeEditable which selects the text as soon as the NodeEditable component mounts was being called before it's parent component's useEffect() had called setBookmark(), so text could node be selected because it wasn't in the document yet.

To avoid the troubles with using setAfterDOMUpdate or similar to "wait until react is done", I had to move responsibility for calling selectElement() up the component hierarchy to ToplevelBlockEditable. So now we only select the element text after it's container element has been inserted into the document via setBookmark(). It's the first time I've had the chance to use useImperativeHandle().

codemirror really makes things 10x more complicated than they would otherwise be, but I hope the comments I've left behind are explanatory enough.

[coveralls]

Coverage increased (+0.2%) to 72.306% when pulling 92d8cd1 on pcardune/fix-591 into 88f004e on master.

[pcardune]

This also fixes #592

[schanzer]

@pcardune "add a useEffect() that focuses the currently focused node when it mounts"

This is not the desired behavior. If a blind user has focus on the "switch to blocks mode" button, they shouldn't have their focus jump to the first block of the editor. The only time focus should jump like that is when they're already in the block editor, and they make an edit that switches focus.

[pcardune]

How about this?

[schanzer]

My understanding of line 250 is that this hook "subscribes" to the focusedNode selector and the node id. Does this mean that any time the focusedNode changes, every node asks if the focused node's id is the same as the node's id? If not, what does it mean?

[pcardune]

yep, that's what this means.

[schanzer]

Perfect! Merging now. Thanks for adding the tests!