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

Decorate XYPlot onMouseEnter and onMouseLeave events #176

Closed
krissalvador27 opened this issue Aug 22, 2019 · 5 comments · Fixed by #182
Closed

Decorate XYPlot onMouseEnter and onMouseLeave events #176

krissalvador27 opened this issue Aug 22, 2019 · 5 comments · Fixed by #182

Comments

@krissalvador27
Copy link
Contributor

We decorate onMouseMove, onMouseDown, onMouseUp, onClick with helpful information like xScale, yScale, xValue and yValue. We should decorate onMouseEnter and onMouseLeave for consistency.

@tanayv
Copy link
Contributor

tanayv commented Sep 27, 2019

Hey @krissalvador27!

I can take this up. Just to clarify: decorating the onMouseEnter and onMouseLeave events would involve adding a binding to the onXYMouseEvent function right?

Let me know if that's what you had in mind, and I can have a PR for this ready. Thanks!

@krissalvador27
Copy link
Contributor Author

Hey @tanayv!

Sorry for the delay, I've been out of office. What you said is right!

Let me know if you need any other support :)

@krissalvador27
Copy link
Contributor Author

Hey @tanayv, do you plan on taking up this issue?

@tanayv
Copy link
Contributor

tanayv commented Oct 8, 2019

Yes, sorry for the delay. I just had one thing I was slightly confused about:

  onMouseMove = this.onXYMouseEvent.bind(this, 'onMouseMove');
  onMouseDown = this.onXYMouseEvent.bind(this, 'onMouseDown');
  onMouseUp = this.onXYMouseEvent.bind(this, 'onMouseUp');
  onClick = this.onXYMouseEvent.bind(this, 'onClick');

  // Decorate onMouseEnter and onMouseLeave
  onMouseEnter = this.onXYMouseEvent.bind(this, 'onMouseEnter');
  onMouseLeave = this.onXYMouseEvent.bind(this, 'onMouseLeave')

  // Possibly redundant?
  onMouseEnter = event => this.props.onMouseEnter({ event });
  onMouseLeave = event => this.props.onMouseLeave({ event });

I couldn't deduce if there was a reason/use case for why the two events were declared in a different way compared to the other events, and I was going through the code base to understand but I haven't been able to do so. If I bind them using the onXYMouseEvent, should I go ahead and remove their original declarations?

@krissalvador27
Copy link
Contributor Author

I believe it's to namespace the event object so that it returns { event: { ... } } as the callback argument. Similar in style to how getMouseOptions adds the event object to the event key.

So yes, I would remove the original declarations :)

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.

2 participants