Previously-focused block steals focus from blocks added by the main inserter

[fullofcaffeine]

Description

This is somewhat of a corner case and I could only reproduce with the paragraph block being the first block on the page. When that's the case, and if has been given focus and some text, and if you try adding other blocks after it, you'll notice that the newly-inserted block won't get focus, the focus will stay (or get back?) to the paragraph input.

Step-by-step reproduction instructions

1. Create a new post or page;
2. Type something in the paragraph block;
3. While leaving the focus on the paragraph block, open the main inserter, search for the "Image" block and click it to add to the page;
4. It will be added after the paragraph block, but the focus will quickly jump back to the paragraph.

I should note that this doesn't seem to happen if the block you add after the paragraph also is a text input. For example, the "Quote" block. Then it seems to get the focus, as expected.

Expected behavior

The last-inserted block should be given and keep the focus. It should be automatically selected after insertion from the main inserter.

Actual behavior

The focus/selection goes back to the paragraph's text input.

Screenshots or screen recording (optional)

Peek.2021-02-10.19-43.mp4

Code snippet (optional)

Not a snippet, but I did try bisecting between GB 9.8.4 and 9.9.1 (the version where I first experienced this issue) and git pointed me to this merge commit as the culprit: #28438.

WordPress information

• WordPress version: 5.7-beta2-50285
• Gutenberg version: 9.9.1 and master (see video above)
• Are all plugins except Gutenberg deactivated? Yes
• Are you using a default theme (e.g. Twenty Twenty-One)? Yes

Device information

• Device: Desktop
• Operating system: Ubuntu Linux 18.04
• Browser: Chrome 87.0.4280.88 (Official Build) (64-bit) / Firefox 86.0b8 (64-bit)

[WunderBart]

It looks like the useFocusReturn’s callback is firing after the focus has been set to a newly inserted block. I think that the PR that caused this regression was actually legit because this issue is specific to the main inserter only. The useDialog component that is used to render the inserter uses the useFocusReturn and it works correctly, just not in the case when a focus is forced on the close dialog event. I think the correct solution to this might be somehow stopping the useFocusReturn callback when a block is inserted, so it doesn’t redirect the focus to what was active before the inserter was open (Paragraph block in our test case). Does that make sense? cc @ockham @noahtallen @brentswisher

[brentswisher]

Yeah, I think so. By default, when a dialog is closed, the focus should return to the element that spawned it when it is closed. The inserter is seeing that as the paragraph in this case. I think you are on the right track but am not sure specifically how to implement that without looking into it further - that PR was the first time I had worked with useFocusReturn 😓

[WunderBart]

Looks like this issue is back in v10.1.0-rc.1. We have an E2E test to catch it, but it seems like it didn't make it to v10.1 (planned for v10.2). Any idea what could be causing this regression? @youknowriad @ockham @fullofcaffeine @rafaelgalani

[WunderBart]

Looks like this issue is back in v10.1.0-rc.1. We have an E2E test to catch it, but it seems like it didn't make it to v10.1 (planned for v10.2). Any idea what could be causing this regression? @youknowriad @ockham @fullofcaffeine @rafaelgalani

Interestingly, that E2E test wouldn't catch it as the focus stealing happens on mobile viewport only (Unless those specs are set to run on all viewports which I'm not aware of). I've confirmed it by running that test on a small viewport (via await setBrowserViewport( 'small' );):

 FAIL  packages/e2e-tests/specs/editor/various/inserter.test.js (17.163 s)
  Inserter
    ✓ shows block preview when hovering over block in inserter (5189 ms)
    ✕ last-inserted block should be given and keep the focus (6474 ms)

  ● Inserter › last-inserted block should be given and keep the focus

    expect(received).toBe(expected) // Object.is equality

    Expected: "core/image"
    Received: "core/paragraph"

      42 |              } );
      43 | 
    > 44 |              expect( selectedBlock.name ).toBe( 'core/image' );
         |                                           ^
      45 |      } );
      46 | } );
      47 | 

      at Object.<anonymous> (specs/editor/various/inserter.test.js:44:32)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   0 total
Time:        17.309 s, estimated 48 s