Reach-ui: Dialog: Allow manual control of focus after dialog closes

[brendancarney]

Dialog uses FocusLock, which returns focus to the last element that had the focus when the dialog is closed. In our case, we actually want to focus something else. Currently we have to work around this by using setTimeout to make sure our focus code runs after Reach UIs.

What do you think about adding support for controlling this focus behavior? I'm not sure exactly what it would look like. A couple options:

• A prop (returnFocus maybe?) to disable the auto focusing.
• Some function passed to onDismiss that allows prevention of the auto focusing.

Thanks!

[briandipalma]

I'm happy to create a PR if we get the go ahead for this as we're also looking for this feature.

[briandipalma]

This is the workaround I used in case others come across this issue. It makes me thing the cleanest API would be for returnFocus to be supported by the Dialog etc components.

function blurActiveElement() {
  setTimeout(function() {
    document.activeElement.blur();
  }, 0);
}

export default function PopupDialog({
  onDismiss,
  isOpen,
  returnFocus,
  children
}) {
  // This function is run either on component unmount or when the two dependent
  // values change.
  // If when the effect was created the Dialog was open and not meant to return
  // focus then we blur the active element. Strictly speaking it would be wrong
  // to run it if only `returnFocus` changed but as the Dialog is still open in
  // that case I see no issue.
  useEffect(
    () => () => {
      if (returnFocus === false && isOpen === true) {
        blurActiveElement();
      }
    },
    [returnFocus, isOpen]
  );

  ...
}

[mdugue]

Any news here? I really enjoy using reach-dialog with it's great details. For me the only missing part is better control on the focus return upon closing it. 😅I can image quite some use cases potentially relevant to many people where you want to opt out of the default behavior. Would you be open to a PR adding a disableAutomaticFocusReturn prop? Any recommendations on that wording / API surface?

Thanks a lot in advance!!

[mdugue]

actually, I just discovered this PR https://github.com/reach/reach-ui/pull/450. @chancestrickland any chances that this could get merged?

[chaance]

I understand why this might be necessary, but I do want to consider the API a bit more before merging #450. Keep in mind that you can always patch this behavior in the mean time if you'd like with https://www.npmjs.com/package/patch-package

[mdugue]

Sure! Any API ideas you want to share and bounce?

[jromerof]

@chancestrickland I second @mdugue as i created the PR and I usually check the reach files to get insights on how to make my own components API
Also thanks for the patch-package suggestion I didn't know about it!

[mdugue]

Hey @chancestrickland any news on that topic? I perfectly understand that you don't want to clutter the API with "single purpose" features, keep it as simple as possible, avoid "impossible props" etc.
Still I think that this is definitely an important feature for many. If you have any thoughts on that functionality, I'd love to discuss them.

[ptcampbell]

Hello, I thought I would share my use case for this request. I am using a menubutton in a persistent navigation. When changing routes via the dropdown, it should focus on the body but my focus method is being hijacked by Reach. I realise this dicussion is about the Dialogue component, but it's basically the same problem.