alexkatz / react-tiny-popover

A simple and highly customizable popover react higher order component with no other dependencies! Typescript friendly.
MIT License
454 stars 121 forks source link

`onClickOutside` triggered for unmounted elements #138

Closed xPapla closed 2 years ago

xPapla commented 3 years ago

It is possible to trigger onClickOutside by unmounting the element that was clicked. I think the way to fix it is to either listen for capturing event or by changing the code to use mousedown and touchstart events here: https://github.com/alexkatz/react-tiny-popover/blob/291df01d28bb4a1849d5f3ebe4945b3735f586b6/src/Popover.tsx#L193-L200

Issue demo:

import React from "react";
import { Popover } from "react-tiny-popover";

const Content = () => {
  const [visible, setVisible] = React.useState(true);

  return (
    <div style={{ background: "red", height: 150, width: 150 }}>
      <button onClick={() => setVisible(!visible)}>Clicking this works</button>
      {visible && (
        <button onClick={() => setVisible(false)}>Broken button</button>
      )}
      Other content
    </div>
  );
};

export default function App() {
  const [isOpen, setOpen] = React.useState(false);

  return (
    <Popover
      isOpen={isOpen}
      content={<Content />}
      padding={10}
      positions={["bottom"]}
      onClickOutside={() => setOpen(false)}
    >
      <button style={{ width: 500 }} onClick={() => setOpen(!isOpen)}>
        Trigger
      </button>
    </Popover>
  );
}
davidjgoss commented 2 years ago

A workaround that might be viable for some use cases:

    <Popover
      isOpen={isOpen}
      content={<Content />}
      padding={10}
      positions={["bottom"]}
      onClickOutside={e => {
        if (e.target.isConnected) {
          setOpen(false)
        }
      }}
    >
      <button style={{ width: 500 }} onClick={() => setOpen(!isOpen)}>
        Trigger
      </button>
    </Popover>

isConnected will be false where the element has been detached from the document, which would be the case in the examples described here where we click something within the popover and it gets unmounted. However, this could also yield false negatives depending on what's outside the popover.