Reach-ui: [Dialog] Add option to disable focus lock

[brendancarney]

I know this title sounds bad, but here's the use case: we build a landing page editor, and one of our templates is going to have a dialog. We need a way for the dialog to be open, but have some controls available in a sidebar that are used to configure/style the dialog. It looks something like this:

Would you be open to a PR that adds a prop to disable the focus lock? We can make it have a scary name and strong wording in the documentation.

Thanks!

[venikx]

How do you plan dealing with focus management and a11y in your use-case when something like disableFocusLock is applied?

Regarding the spec around Dialog/Modals (it's a bit unclear to what exactly the difference is), it's valid for a "Modal" to not have any interactive elements on the background.
They also show an example of a "Calender Dialog" implementation, where opening the calendar, moves focus to the calender, but other elements on the page are still intractable. However interacting with one of these elements "other than the ones inside the Calender Dialog" results in the Dialog losing focus and giving it back to where it left of or moves it to next intractable element.

From an a11y standpoint this makes sense. If I interact with a Dialog of some sorts, I want focus to be trapped inside the Modal until I tell it be closed (ESC according to the spec) or by clicking away/interacting with another element on the page.

Not sure if the disableFocusLock will result in good a11y practices, which is what reach-ui tries to aim for. The material-ui team also provides a prop like this, but recommends against using it, since it breaks a11y.

[hedinne]

It sounds like you have very specific use-case. Basically two sites (the editor and the target site) but in the same document (not using iframe). I would say it would be more reasonable for you to just fork the dialog and change it rather then change if for everybody. Adding this flag both leads to more people to misusing it and adds complexity to the dialogs API.

p.s. If you disable the focus lock, then it will be much harder for people who use the keyboard to use your editor to navigate the editor+the site. If you can, try to keep the dialog as-is and add the target site to an Iframe.

[venikx]

Agreed, disabling the focus lock would be against the WAI-ARIA best practices, since it the lib would make it easy to opt out of the a11y capabilities. To me that'd be weird, since that's the main point of the lib, provide accessible defaults.

[brendancarney]

@venikx I understand how a dialog is generally supposed to work in terms of accessibility. This is not the normal use case, as I explained in the issue. The goal here is to ultimately use reach ui on the pages our tool builds, providing an accessible experience for visitors of those pages.

To me that'd be weird, since that's the main point of the lib, provide accessible defaults.

It would still provide accessible defaults.

@hedinne it may be too much of a specific use-case, and if the maintainers of reach-ui don't want to support it, I totally understand. Although moving everything to an iframe would help with this dialog situation, it wouldn't do much to improve accessibility in our case, but I appreciate your suggestion.

[venikx]

@brendancarney It's not up to me, but personally I believe it goes against the "spirit" of the lib.

Yes, you are giving accessible defaults, but the whole point (at least that's how I interpreted it) of reach-ui is make the web more accessible by not preventing application developer from creating inaccessible components.
The escape hatch you are suggesting would defeat that purpose, since you'd be able to create inaccessible dialogs again.

[brendancarney]

Another implementation idea that I think might be better: expose @reach/focus-lock. If the dialog is already in a <FocusLock>, then it won't wrap itself in one. Then, the user can specify how the <FocusLock> behaves. This also has the advantage of provided another useful tool straight from reach-ui.

I think this is better because it doesn't add an awkward prop, and the API structure makes it so that only someone who knows what they're doing is likely to use it.

Example:

<FocusLock disabled>
   <Dialog> // already wrapped in FocusLock, let that take control
   </Dialog>
</FocusLock>

[jflanaganuk]

Has there been any update to this issue? I've just ran into a similar problem to original poster?

I would imagine we can fix this by just checking if "initialFocusRef" is defined or similar?
https://github.com/reach/reach-ui/blob/3079d56a18e1e2ff04921ae97502661bedfbdc9f/packages/dialog/src/index.js#L56

autoFocus={Boolean(initialFocusRef)}

So if i pass "null" to initialFocusRef i get the desired behaviour of nothing focused.

[darklightblue]

Hi. I encountered the same issue - any updates on this one?

In my case I am using DialogContent for my Modal (which is being reused).

Most of my input components have a focus UI and there are some modals that I don't want have an auto focus component because it makes the user to pick whatever the first focused component is.

[jflanaganuk]

Hi. I encountered the same issue - any updates on this one?

In my case I am using DialogContent for my Modal (which is being reused).

Most of my input components have a focus UI and there are some modals that I don't want have an auto focus component because it makes the user to pick whatever the first focused component is.

Personally, I ended up just using https://www.npmjs.com/package/patch-package and patching what I needed (set autofocus to false in index.js)

[chaance]

I've been thinking about this for a while and trying to find the least footgun-prone API we could introduce to help solve for some niche cases. I don't think disabling the focus lock is the right answer because it's crucial for the vast majority of valid uses.

A couple of thoughts:

One potential solution is to create a faux-modal that isn't treated like a normal modal in its DOM implementation. You probably still want to aria-hidden and tabindex="-1" on elements that are underneath the modal, but unless it's in an iframe it probably shouldn't be perceivable as a modal to assistive devices if you have content in the sidebar you can still control and access. You also might not want the popup or anything inside of it to steal focus if you open it from the sidebar outside of that frame. This would take some testing, but I think it's preferable to changing our API to allow for practices that would likely be harmful and easily abused.

For the OP, this could then be implemented something like this:

function MyDialog(props) {
  let { inPageEditorView } = useContext(appContext);
  return inPageEditorView ? (
    <FakeDialog {...props} />
  ) : (
    <Dialog {...props} />
  )
}

Another thought I'm exploring: React Focus Lock has the concept of focus groups. Essentially you could apply a group prop to the dialog, then wrap your own component in a FocusLock component with the same group prop. That would allow the focus to be locked between those two nodes, but it still wouldn't stop the dialog (or one of its children) from stealing focus. I'll have to test this further to see what the accessibility implications are, but it feels much better than simply disabling the lock.

Closing this for now as I'm unlikely to implement the suggestions here, but I may create a new branch for exploring this concept a bit further. Feel free to patch the package in the mean time if you truly need it. https://www.npmjs.com/package/patch-package