[Focus lock breakage after updating React to V17]: Unable to get focus inside dialog content( react portal) which is being created from focus-lock content

[ramareddy12345]

Hello,

We are using focus-lock library in our application. We recently updated react to V17 from V16+.

We have observed that because of change in react-dom event flow (facebook/react#18195) now focus lock events order is getting changed resulting in focus issue.

Earlier: Focus order as follows
focus event (lock.js)
focus-in event(trap.js)
blur event (trap.js)

Now: Focus order as follows
focus-in event(trap.js)
focus event (lock.js)
blur event (trap.js)

And also we have observed that, with react-dom V17 if we change focus-in event handler(trap.js) from capture to bubbling phase then event order triggered same like earlier and issue got fixed.

I replicated the scenario in codesandbox and attaching the same.

Steps to reproduce:

1. Click on below sandbox link
2. Click on Show dialog button which creates dialog content by using react portal.
3. Try to focus on input elements inside opened dialog

Working - With React V16
https://codesandbox.io/s/react-focus-lock-forked-z5pk2

Not working - With React V17
https://codesandbox.io/s/react-focus-lock-forked-t8pzl

Due to this issue user has to click twice on the input controls which are part of dialog to get focus.

Could you please help us on this usecase.

[theKashey]

Oh, welcome to the shiny new world.

The proper, cross-version compatible fix might take a while, but this is something that had to be made in order to support many existing UI patterns and verify the results. Gonna start with reading some docs and PR you've linked. So bear with me and feel free to participate.

Meanwhile, you can use shards(array of refs) to add "other DOM nodes" FocusLock should consider as a part of itself - it more or less independent of the used React version but require almost manual wiring, which is not always possible.

[LJKaski]

Hello, and first of all thanks for creating such an awesome library! I wanted to check on the status of this issue as there have been no comments since January. Any idea on possible schedule? A large amount of work for a workaround hangs in the balance, so any kind of estimate (or lack thereof), would help me decide if the workaround is worth it.

Thank you in advance.

[theKashey]

Unfortunately, I still don't get a moment to work on this, but I am looking forward to work on this part of ecosystem soon as "we" are on React 17 as well and soon will hit the same issue.

[LJKaski]

Thank you for the reply. I think we'll have to build a workaround for now then :)

[johnathanludwig]

I looked into this a little bit based on the research from @ramareddy12345. The changes made in johnathanludwig@63f84fa fix the issue for our use case and tests continue to pass.

However, I do see test failures when I upgrade this library to use react 17. These test failures seem to occur regardless of the change made and are related to autofocus. In our case with react 17, autofocus still seemed to be working properly.

I'm not really sure of the test issues or how to fix them. If you are willing to accept it I can open a PR for the referenced change which should still pass tests until this library uses react 17 for testing.

Here is an output of the failures:

 1) react-focus-lock
       FocusLock
         portals
           false auto test:
     AssertError: expected spy to be called once but was called 0 times
      at Object.fail (node_modules/sinon/lib/sinon/assert.js:106:21)
      at failAssertion (node_modules/sinon/lib/sinon/assert.js:65:16)
      at Object.calledOnce (node_modules/sinon/lib/sinon/assert.js:91:13)
      at Context.<anonymous> (_tests/FocusLock.spec.js:838:22)
      at processImmediate (internal/timers.js:461:21)

  2) react-focus-lock
       FocusLock
         portals
           Should handle portaled content:

      Uncaught AssertionError: expected 'button-action' to equal 'i am portaled'
      + expected - actual

      -button-action
      +i am portaled

      at Timeout._onTimeout (_tests/FocusLock.spec.js:870:58)
      at listOnTimeout (internal/timers.js:554:17)
      at processTimers (internal/timers.js:497:7)

  3) react-focus-lock
       FocusLock
         portals
           Should handle auto portaled content:

      AssertionError: expected 'button-action' to equal 'i am portaled'
      + expected - actual

      -button-action
      +i am portaled

      at Context.<anonymous> (_tests/FocusLock.spec.js:902:56)
      at processImmediate (internal/timers.js:461:21)

[danielstreit]

@theKashey Any thoughts on @johnathanludwig 's fix linked above?

Is the capture absolutely necessary? And if so, why?

[theKashey]

So a few things:

• the fix is valid. I will double-check how it works with React 16, but for 17 this is how it should be due to change the way React 17 attaches to page events.
  • I will release an update shortly after it
• tests are failing due to no events in portals has been recorded or triggered. Probably this caused by React 17 behavior not being replicated by (nonofficial) enzyme-17 adapter. Probably here is a time to update very old codebase to something newer.

Also worth noting that I do not sure why capture was used. I have a few ideas, but none of them could verify the actual "need" to use capture. dom-focus-lock, which I wrote later, does not have it. While vue one, developed in parallel, has.
Yet again, I promise myself to start leaving notes about even obvious things to my future self as well as any other who ever read my code (cc @Marais)