theKashey / react-focus-on

🎯 Solution for WAI ARIA compatible modal dialogs or full-screen tasks, you were looking for
MIT License
336 stars 14 forks source link

onClickOutside doesn't prevent reopening #33

Closed zoontek closed 4 years ago

zoontek commented 4 years ago

Hi @theKashey πŸ‘‹

I used this library on a bunch of projects, it's really great except for one small thing: onClickOutside. I struggle to make it works on mobile because it doesn't prevent the initial event.

To explain my pain, suppose that we have a Profile Button which open a profile form modal:

 β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
 β”‚                                                 β”Œβ”€β”€β”€β”€β”€β”€β”€β”β”‚
 β”‚                                                 β”‚Profileβ”‚β”‚
 β”‚                                                 β””β”€β”€β”€β”€β”€β”€β”€β”˜β”‚
 β”‚               β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚    Profile form modal    β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β”‚                          β”‚               β”‚
 β”‚               β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜               β”‚
 β”‚                                                          β”‚
 β”‚                                                          β”‚
 β”‚                                                          β”‚
 β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜

If I choose to close my modal on click outside, it's easy, but it will not works if I click on the profile button (it will close on reopen the modal immediately).

theKashey commented 4 years ago

Yeah, I hate this. Clicking external buttons, expecting this modal to be closed, is reopening it. It's working quite simple - you click, the click got caught on capture phase, DOM got unblocked, and the initial click lands on the element you just clicked.

Known solutions:

zoontek commented 4 years ago

prevent during onClickOutside did not worked for me.

What I finally found (using portals):

<div id="app-root">
  <!-- … -->
</div>

<div id="modals-root">
  <FocusOn>
    <!-- a div that cover the page entirelly -->
    <div style={{ position: "absolute", top: 0, left: 0, right: 0, bottom: 0 }} onClick={handleClose} />

    <div id="the-modal">
      <!-- … -->
    </div>
  </FocusOn>
</div>
theKashey commented 4 years ago

A bit complicated, but hopefully it works :+1:

pablen commented 3 years ago

I'm trying something like this. Basically I check if the click was inside the trigger element before closing the modal. If that's the case I do nothing and let the trigger element handler toggle the modal visibility.

const [referenceElement, setReferenceElement] = React.useState(null)
const [isVisible, setIsVisible] = React.useState(false)

<button
  aria-expanded={isVisible}
  onClick={() => setIsVisible(s => !s)}
  ref={setReferenceElement}
>
  Click here
</button>

<FocusOn
  onClickOutside={event => {
    if (
      !referenceElement.contains(event.target) &&
      referenceElement !== event.target
    ) {
      setIsVisible(false)
    }
  }}
>
  <Modal />
</FocusOn>

Seems to work fine, but I would appreciate any heads up.

theKashey commented 3 years ago

@pablen your code can be simplified a little:

const referenceElement = React.useRef(null);
const [isVisible, setIsVisible] = React.useState(false)

<button
  aria-expanded={isVisible}
  onClick={() => setIsVisible(s => !s)}
  ref={setReferenceElement}
>
  Click here
</button>

<FocusOn
  shards={[referenceElement]} // πŸ‘ˆ now that button is considered as a part of FocusOn
  onClickOutside={() =>  setIsVisible(false)}
>
  <Modal />
</FocusOn>
pablen commented 3 years ago

Love it. Thanks!