Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: pass event to handler callback in useOutsideClick #1998

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

lolmaus
Copy link
Contributor

@lolmaus lolmaus commented Dec 17, 2024

We're using useOutsideClick to close a modal.

The modal renders a menu popup with a tethering mechanism (Popper.js or similar). The popup is rendered not inline, but on the root level at the very end of the <body> content. As a result, when a menu item is clicked, the useOutsideClick callback is triggered and the modal is closed.

Conceptually, the menu popup is part of the modal, so selecting an item in the menu should not close the modal.

This can be easily resolved by checking that the click has been made on the actual page and not a popup which is not part of the page:

const handler (e: MouseEvent | TouchEvent) => {
  if (
      e.target instanceof HTMLElement // Type narrowing to allow using `e.target.closest()`
      && e.target.closest('#my-app') // Ignore clicks on popups. Only respond to clicks on the actual page.
  ) {
    closeModal();
  }
}

useOutsideClick({ref: foo, handler});

But unfortunately, the useOutsideClick hook does not pass the event.

This PR enables passing of the event.

PS Me on Staff: https://nda.ya.ru/t/BGTaa7T67AUVGa

@lolmaus lolmaus requested a review from NikitaCG as a code owner December 17, 2024 16:10
@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@gravity-ui-bot
Copy link
Contributor

Visual Tests Report is ready.

@lolmaus lolmaus changed the title Pass event to handler callback in useOutsideClick feat: Pass event to handler callback in useOutsideClick Dec 17, 2024
@lolmaus lolmaus changed the title feat: Pass event to handler callback in useOutsideClick feat: pass event to handler callback in useOutsideClick Dec 17, 2024
@@ -2,7 +2,7 @@ import React from 'react';

export interface UseOutsideClickProps<T> {
ref: React.RefObject<T>;
handler?: () => void;
handler?: (e: MouseEvent | TouchEvent) => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make e optional e?. It looks like breaking change right now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it won't be a breaking change. It's like onClick in Button where you can skip this argument if there is no need to use it

@NikitaCG NikitaCG requested a review from korvin89 December 24, 2024 12:28
@lolmaus lolmaus force-pushed the use-outside-click-pass-event-to-handler branch from 6f94a90 to 8e365cc Compare December 24, 2024 13:32
@lolmaus lolmaus merged commit 3097c80 into main Dec 24, 2024
6 checks passed
@lolmaus lolmaus deleted the use-outside-click-pass-event-to-handler branch December 24, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants