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

[EuiFlyout] onClose invoked with unexpected parameter #2894

Closed
pickypg opened this issue Feb 21, 2020 · 5 comments · Fixed by #4505
Closed

[EuiFlyout] onClose invoked with unexpected parameter #2894

pickypg opened this issue Feb 21, 2020 · 5 comments · Fixed by #4505

Comments

@pickypg
Copy link
Member

pickypg commented Feb 21, 2020

The definition for onClose as part of EuiFlyout is () => void. I was taking advantage of this by having my not-close button invoke the same method with an optional parameter.

However, I was surprised when certain closure's were triggering issues in my code. Upon inspection, it kind of became clear that I was receiving the events from the button clicks:

onClick={onClose}

This is easy to workaround: just pass in the function as () => actualCallback(), but it was definitely unexpected.

@Dilshaad21
Copy link

@pickypg Can you please explain the issue or how can I reproduce this.

@pickypg
Copy link
Member Author

pickypg commented Feb 22, 2020

Something along the lines of:

onComplete(data?: object) {
  if (data) {
    console.log('This also fires for certain uses of [onClose]');
  } else {
    console.log('As does this [onClose]');
  }
}

render() {
  return (
    <EuiFlyout
      onClose={this.onComplete}
      ownFocus
      // ...
    />
  );
}

When pressing the close button, it will pass the event data through to

@raghav4
Copy link

raghav4 commented Feb 22, 2020

@pickypg can I work on this?

@raghav4
Copy link

raghav4 commented Feb 22, 2020

Update:
@pickypg I tried reproducing whatever you've said, but couldn't encounter the issue, can you please elaborate it a little more?

@thompsongl
Copy link
Contributor

We do need to makes some changes here.
Two choices:

  • Keep () => void as the type and eliminate the event parameter coming from EuiButtonIcon
  • Change to (Event) => void and pass the event parameter through in all cases.

The second option requires making to changes to EuiOverlayMask, so I'd likely opt for the first option.

@cchaos cchaos changed the title EuiFlyout onClose invoked with unexpected parameter [EuiFlyout] onClose invoked with unexpected parameter Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants