Reach-ui: Dialog's focus lock prevents iframe from having focus

[gnapse]

🐛 Bug report

Imagine a focus-locked area of a web page (in this case a reach-ui dialog). Then we open an iframe that's actually mounted outside of the focus-locked area, though visually it appears as a dialog itself (though this iframe is not using @reach/dialog, the part that is focus-locked is what is really a @reach/dialog in this example).

See codesandbox example linked to below.

Current Behavior

The focus-locked @reach/dialog instance is too aggressive in keeping the focus for itself, preventing the iframe from having it.

Expected behavior

I understand the current behavior. But I'd like that @reach/dialog would allow me to tell it otherwise.

Reproducible example

CodeSandbox Template

Try to type inside the Wikipedia search field that opens inside the iframe.

Suggested solution(s)

Maybe by exposing the whilelist prop of the react focus lock somehow.

Additional context

Related to #536. That one fixed the case where the focus-locked area is inside an iframe, and was preventing from accessing things outside the iframe. However, this case is when the focus locked area is outside the iframe, and prevents focus from being set to things inside an iframe.

Your environment

| Software | Name(s) | Version |
| ---------------- | ------- | ------- |
| Reach Package | dialog | 0.10.3 |
| React | | 16.13.1 |
| Browser | Chrome, Firefox | latest |
| Assistive tech | | |
| Node | | |
| npm/yarn | | |
| Operating System | | |

[chaance]

@gnapse I'm a little confused by your example. If your iframe looks like a dialog but isn't a proper dialog, isn't that kind of a problem in itself? Understanding here that it's likely a simplified version of a real problem, but I'm cautious not to encourage or enable dangerous configuration via props so I want to understand the use case a bit better.

[gnapse]

The real use case is that from an application dialog the user can open a Dropbox or a Google Drive file picker. These are handled by their respective development kits, and what both (understandably) do is to open their app in an iframe, but they make it look like something that's floating on top of your app as a dialog, very much using the same markup used in the example I created. I understand it's not the best situation to deal with, but it is what it is.

[chaance]

I'm leaning towards a bypassFocusLock prop TBH. I understand these special cases are sometimes unavoidable but it's less desirable in my opinion to have a bunch of pass-through props to try and deal with weird edge cases like this. I'm hesitant because it's never a good idea to truly bypass the focus lock, but if you want to configure your own you should be able to do so by building and composing it to fit your needs.

<DialogOverlay bypassFocusLock>
  <MyFocusLock>
    <DialogContent>
      {children}
    </DialogContent>
  </MyFocusLock>
</DialogOverlay>

[gnapse]

That seems like a very good idea to me. Indeed better than making DialogOverlay receive some sort of focusLockProps that not only look weird and breaks composition, but prevents users from, as you said, roll out their own focus lock if they need to (as we do).

With something like this in place, we could tell reach-ui to bypass focus lock, and we'll roll our own with the whitelist prop.

Would you mind me taking a shot at making the contribution myself?

Also, should it be bypassFocusLock or maybe a focusLock={false} and making it true by default? I seem more inclined to the latter, but can do it however you prefer.

[chaance]

@gnapse The contribution is welcome!

I'd probably go with dangerouslyBypassFocusLock, set to false by default. We definitely don't want to encourage its use in the majority of cases. I'd like to add some documentation about why this is dangerous and should be avoided in most cases, and include an example showing how a custom focus lock can be added to the composed dialog. Happy to help with this if you run into any issues.

[gnapse]

@chancestrickland I opened a PR. I'm sure it'll have some things to be improved or added (e.g. I did not add new tests or had to modify any, but I also did not see any tests about focus lock). I'm also not sure if the PR should be involved with documenting this in the website or elsewhere. I added a relatively acceptable documentation to the prop jsdoc, and the examples on how to use the new feature responsibly are in a new storybook example.