Gutenberg-mobile: Moving the current block down sets the caret on the wrong block

[daniloercoli]

Moving a block down would move the focus on the wrong block.
This is only happening on Android, I̶ ̶t̶e̶s̶t̶e̶d̶ ̶i̶O̶S̶ ̶a̶n̶d̶ ̶i̶t̶ ̶s̶e̶e̶m̶s̶ ̶t̶o̶ ̶w̶o̶r̶k̶ ̶j̶u̶s̶t̶ ̶f̶i̶n̶e̶. The focus is kept on the current block, the one I just moved down.

Steps to repro:

• Start the demo app
• Select a RichText block like Hello World
• Tap the down arrow on the toolbar
• The block is correctly moved down
• But the focus is now on the first block in the list Welcome to GB

With the same steps above try to move the code block. The focus is moved to the first RichText block above.

Note: I also see a warning in the AS log:

2018-12-03 12:20:40.502 6836-6836/com.gutenberg E/unknown:NativeViewHierarchyManager: Unable to update properties for view tag 693
    com.facebook.react.uimanager.IllegalViewOperationException: ViewManager for tag 693 could not be found
        at com.facebook.react.uimanager.NativeViewHierarchyManager.resolveViewManager(NativeViewHierarchyManager.java:107)
        at com.facebook.react.uimanager.NativeViewHierarchyManager.updateProperties(NativeViewHierarchyManager.java:134)
        at com.facebook.react.uimanager.UIImplementation.synchronouslyUpdateViewOnUIThread(UIImplementation.java:338)
        at com.facebook.react.animated.PropsAnimatedNode.restoreDefaultValues(PropsAnimatedNode.java:76)
        at com.facebook.react.animated.NativeAnimatedNodesManager.restoreDefaultValues(NativeAnimatedNodesManager.java:337)
        at com.facebook.react.animated.NativeAnimatedModule$18.execute(NativeAnimatedModule.java:350)
        at com.facebook.react.animated.NativeAnimatedModule$2.execute(NativeAnimatedModule.java:141)
        at com.facebook.react.uimanager.UIViewOperationQueue$UIBlockOperation.execute(UIViewOperationQueue.java:567)
        at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:894)
        at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1001)
        at com.facebook.react.uimanager.UIViewOperationQueue.access$2400(UIViewOperationQueue.java:46)
        at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1061)
        at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
        at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:134)
        at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:105)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:909)
        at android.view.Choreographer.doCallbacks(Choreographer.java:723)
        at android.view.Choreographer.doFrame(Choreographer.java:655)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:897)
        at android.os.Handler.handleCallback(Handler.java:789)
        at android.os.Handler.dispatchMessage(Handler.java:98)
        at android.os.Looper.loop(Looper.java:164)
        at android.app.ActivityThread.main(ActivityThread.java:6541)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)

[daniloercoli]

Hey @Tug, would you mind to take a look at this issue here? It seems to be related to a wrong selected block index stored in props this.props.selectedBlockIndex when moving the block up/down.
I thought it was an issue with Android only, but re-tested it on iOS, and the issue is there as well, even though it's less noticeable.

Added a log right before the action that does move the block, and the index of the selected block it seems "behind" by one step of the actual position of the just moved block.

    moveBlockUpAction = ( clientId ) => {
        console.log(Date.now() + ' ' + this.props.selectedBlockIndex);
        this.props.onMoveUp( clientId, this.props.rootClientId );
        console.log(Date.now() + ' ' + this.props.selectedBlockIndex);
    };

    moveBlockDownAction = ( clientId ) => {
        console.log(Date.now() + ' ' + this.props.selectedBlockIndex);
        this.props.onMoveDown( clientId, this.props.rootClientId );
        console.log(Date.now() + ' ' + this.props.selectedBlockIndex);
    };

[Tug]

Spent some time investigating this and even though I could see some unexpected store modifications at first, I'm now unable to reproduce now even after reverting my changes...
I'll try to come back to it when I can!

[daniloercoli]

Well, the issue does happen when you move down the block @Tug . Just re-tested master to make sure i was working on the correct hash, and it is still there:

[Tug]

Oh interesting, I haven't seen this behavior!
I tested on a real android device and each time I would click on down, it would blur and the cursor would not be visible in any other block.

[daniloercoli]

I also tested on iOS simulator and the behavior is slightly different...sometime the caret is visible on the top block, and sometime it's not. This is definitely a sign that something under the hood is not working as expected.

[daniloercoli]

I've investigated this a bit more, and it seems - like @Tug mentioned above - that the problem on real devices is less noticeable than on emulators. I guess the caret is still available on emulators since the system sees an external keyboard attached to the device.

Recapping (as of today):

• On real device moving down a block dismisses the keyboard and the caret disappear.
• Moving up a block instead does work fine.

Moving this ticket out of alpha.

[hypest]

On real device moving down a block dismisses the keyboard and the caret disappear.

Maybe we still need this fixed in the Alpha since we're focusing on the writing flow? Can you elaborate on move to Beta @daniloercoli? Thanks!
cc @koke

[koke]

We'll want to fix this one soon, but losing focus when reordering blocks seem minor for the alpha.

If it happened while splitting a block on enter, for instance, it would be more disruptive, but I see reordering blocks as a different _mode_ than writing, so losing focus isn't that big of a deal.

[daniloercoli]

losing focus when reordering blocks seem minor for the alpha.

It seems that GB-web does lose the the focus from the editing block when moving it up/down.

See the caret disappear from the edited block when I moved it down. Tapping on the keyboard also does not add new content to it. Should we mimic the same exact behaviour?

[hypest]

👋 @youknowriad , is that by design perhaps on Gutenberg-web? https://github.com/wordpress-mobile/gutenberg-mobile/issues/311#issuecomment-446583290

[koke]

Can we test this again? It'd be fine to leave out for the beta as long as the caret disappears when focus is lost.

[youknowriad]

@hypest I don't think that's by design, when clicking buttons in browsers, the behavior can be different: in some browsers the button receives the focus, in others it doesn't.

I feel like the best behavior would be to keep the caret at the same position but I'm not sure what a11y implications this could have.

[hypest]

keep the caret at the same position

Thanks @youknowriad! Slightly confused by the "same position" though: do you mean same as in stick with the moving block so caret follows the block as it moves? Or "same" as in the caret stays at the (block) list index it was and doesn't follow the moving block?

[youknowriad]

I was thinking if the block being moved is the selected block, just keep the caret at the same position in this block (the block moves obviously). I'd defer to the designers though for a better input on the expect behavior @mapk @jasmussen.

[jasmussen]

This is a really good question. The problem here is that the caret position is tied to _focus_. Which means when you click the "move down" button, or even tab there and press Space, you move focus from where it was in text, to the block mover button. This would likely also explain why the soft keyboard closes when the mover is being used, because text no longer has focus.

I don't know that this is an ideal situation, so I hesitate to say it's by design. But we have to be extremely careful to mess with what _actually_ has focus, because I might want to use the tab key to set focus on the up mover, then press Space a bunch of times to move that block way to the top. So we can't just store the caret position and move focus back there as soon as you've moved a block.

Can we do this?

Desktop: store the caret position if you move the selected block, and if you start typing again, or use the arrow keys, we restore focus and the caret position first.

Mobile: Don't close the soft keyboard if it was open and the block mover was used, and if user starts typing again, restore focus and caret position.

Specifically https://github.com/wordpress-mobile/gutenberg-mobile/issues/311#issuecomment-446274155 looks buggy — in that GIF, the block mover should have focus, but suddenly there's a caret in the Heading block above instead. Is any focus handling going on here?

[hypest]

Let's completely remove the caret and focus from the editing component itself, since the user just tapped on the arrow button itself. We can revisit if the end results still feels wrong. cc @daniloercoli as you're currently assigned with the ticket.

[daniloercoli]

The problem reported here is also available in other testing scenario. See the video here: https://cloudup.com/cLtbRC7sr9l

To reproduce:

• Start the demo app
• Tap on a para block (that's not the first one on the list)
• Tap on the Page break block
• ---> The caret is moved to the first EditText available on the screen.
  X------> The problem doesn't happen if you don't tap before on a EditText powered block.

It seems that on Android when there are EditText(s) on the screen, one of those needs to have the focus/show the caret.

Back to the original testing steps:
I tried to "restore" the focus to the original EditText after the block is moved down, by using TextInputState with not luck. In details:

• Before the block is moved down TextInputState.currentlyFocusedField(); returns the correct ID of the block
• After the block is moved down TextInputState.currentlyFocusedField(); does return null, even though the caret is on the first block of the list.
• Anyway i tried to "restore" the focus back to the original focus by calling TextInputState. focusTextInput( ID of the block); but it didn't work.

I was able although to hide the keyboard when the block is moved down, that is just a small step forward, but makes the UI flow way better.
Will try to set the focus to the parent/root view, this should help on removing the focus and hiding the caret from any EditText.

[daniloercoli]

802 is merged and it does help on mitigating this problem, since it added the code that closes the keyboard when a block is moved Up/Down and making the writing flow a bit better.

[koke]

The problem here is that the caret position is tied to _focus_. Which means when you click the "move down" button, or even tab there and press Space, you move focus from where it was in text, to the block mover button.

@jasmussen This makes sense, but it's also specific to the implementation of focus on the web. Both iOS and Android will keep the keyboard focus separate from the accessibility focus when VoiceOver/Talkback are enabled:

I went to test some things on the web (using latest Safari) and now I'm more confused 😂

In a paragraph block, I can click on the block menu and the cursor is still visible and typing continues writing on the paragraph. From this I assume that a click event doesn't immediately take focus away from where the cursor is.

I can navigate with tabs to different formatting buttons and apply styles. The selection and cursor are not visible while doing so, but the cursor is in the right position if I tab my way back to the block and start typing. This makes me think that preserving cursor position is doable, although it might be complicated.

[koke]

802 is merged and it does help on mitigating this problem, since it added the code that closes the keyboard when a block is moved Up/Down and making the writing flow a bit better.

Moving this to future, since the current behavior seems good enough for 2.0