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

Overlay Mask child element onClick event bubble up #3429

Closed
TAYTS opened this issue May 6, 2020 · 5 comments
Closed

Overlay Mask child element onClick event bubble up #3429

TAYTS opened this issue May 6, 2020 · 5 comments

Comments

@TAYTS
Copy link
Contributor

TAYTS commented May 6, 2020

Hi, may I know is it possible to add a check on the event target for the OverlayMask component to prevent onClick getting triggered by the bubbling up of the child element's click event?

The use case is that I was trying to allow both the EuiOverlayMask and EuiModal's close button to close the modal. However, when I tried to submit the modal form, the EuiOverlayMask's onClick got triggered by the bubbling of the click event from the modal submit button.

if (onClick) {
this.overlayMaskNode.addEventListener('click', onClick);
}

Suggestion:

if (onClick) { 
  this.overlayMaskNode.addEventListener('click', (e) => {
    if (this == e.target) {
      onClick();
    }
  }); 
} 
@chandlerprall
Copy link
Contributor

I think the solution depends on:

@cchaos how do we want consumers to think about the overlay mask:

  • it wraps the modal content in the DOM (how it actually works)
  • it is independent from the content (maybe we change it to be siblings in the future)
  • consumers should never care

If the first, the answer here might be for the consumer to stopPropagation() on those clicks, or to check if the event target is a child node of the overlay.

The second two (and maybe we still do it for the first) would require the proposed solution within EUI

@TAYTS
Copy link
Contributor Author

TAYTS commented May 11, 2020

@chandlerprall @cchaos
May I know is there any updates regarding this issue?

@cchaos
Copy link
Contributor

cchaos commented May 11, 2020

I ran around in circles about the correct "implementation" for the behavior but couldn't come up with a solid solution so instead I'll just describe the expected behavior:

Consumer provides onClick to EuiOverlayMask with children, the children should not receive the close action from the overlay, (no matter if the content is interact-able or not). Meaning, even if the overlay's children is simple text, we need to allow mouse interactions on it without triggering the close event.

So this seems to me, that the component should manage when to perform the onClick.

@chandlerprall
Copy link
Contributor

@TAYTS with all of that, we'll take your suggestion with one modification: will need to compare this.overlayMaskNode === e.target instead of this. Would you like to put a PR together for the change?

@TAYTS
Copy link
Contributor Author

TAYTS commented May 12, 2020

@chandlerprall I have created the PR(#3462)

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

No branches or pull requests

3 participants