tailwindlabs/headlessui

[Bug]: Dialog errors when used in a React routing framework

matt-koevort opened this issue · 9 comments

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

1.2.0

What browser are you using?

Firefox

Reproduction repository

React Router: https://codesandbox.io/s/mystifying-pond-0r5jv?file=/src/App.js
Reach Router: https://codesandbox.io/s/jovial-euler-92zzn?file=/src/App.js

Describe your issue

When using either of the above routing frameworks, when navigating to a different page that renders a Dialog, the Dialog portal seems to get removed - resulting in the error:

There are no focusable elements inside the <FocusTrap />

To reproduce, one can observe the portal in the DOM on refresh, and on clicking a link to navigate to another page (which contains a modal) the portal gets removed from the DOM.

No response

I also had the same bug. temporarily, the error was prevented by delete the code that removes the root portal. novonetworks@232dbe3#diff-8d53567b7a4f03bfcb36cc7efc2ad6a05c7f03dc92c89c90e906d997c8ea74f1

You can test it by yarn add 'https://gitpkg.now.sh/novonetworks/headlessui/packages/%40headlessui-react?232dbe3afad7b79b826f964c12d819232d4c5e50' to your local project. The code sandbox was blocked by CORS and couldn't be shared.

I am encountering this issue on nextjs 10.2.3 with @headlessui/react 1.2.0.

I have a few dialogs/modals on various pages and after changing routes with next/link <Link>, the headlessui-portal-root element is removed from the dom. This seems to correlated strongly with the error.

Hi all,

I made a small reproduction demo repo for this issue using next, tailwind, and @headlessui/react.

Edit:

I did end up finding a workaround. Adding the Dialog on mount seems to help this situation. See this branch in the same repo above for an example.

ldmsh commented

SSR playing trickery here. This hook solved it for me:

// useHasMounted.ts

export default function useIsMounted() {
  const [isMounted, setIsMounted] = React.useState(false);
  React.useEffect(() => {
    setIsMounted(true);
  }, []);
  return isMounted;
}

Use it like so

// page.tsx (NextJS)

import useIsMounted from '@hooks/useIsMounted'

 const isMounted = useIsMounted()

  if (!isMounted) {
    return null
  }

I think I've found a simpler solution (without the hook) to this problem that works just as well. Instead of always trying to render the dialog, you can conditionally render it only when it is open by putting a guard in front of it.

This would look something like this (Note: this tip is taken from the Reach UI dialog which has a similar API. See: https://reach.tech/dialog/)

function Example(props) {
  const [showDialog, setShowDialog] = React.useState(false);
  const open = () => setShowDialog(true);
  const close = () => setShowDialog(false);

  return (
    <div>
      <button onClick={open}>Show Dialog</button>

      {showDialog && (
        <Dialog onClose={close}>
          <p>
            I don’t use <code>isOpen</code>, I just render when I should and not
            when I shouldn’t.
          </p>
          <button onClick={close}>Okay</button>
        </Dialog>
      )}
    </div>
  );
}

This way it will not be rendered on the server and the issue above does not occur. Hope this helps!

Repro using the CodeSandbox above: https://codesandbox.io/s/blazing-darkness-syvyv?file=/src/Page2.js

Hey! Thank you for your bug report!
Much appreciated! 🙏

This should be fixed and is available in the latest version, just update to the latest version and it should start working. Here is an updated codesandbox: https://codesandbox.io/s/zen-phoebe-hzy7i

Amazing, thanks for the awesome work @RobinMalfait! 👏

This issue is still exist on Vue using Vue Router.