Skip to content

Conversation

n-riesco
Copy link
Contributor

@n-riesco n-riesco commented Mar 21, 2017

With this PR, listeners to plotly_click, plotly_hover and plotly_unhover receive the original mouse event (that triggered the event) in data.event.

Implements #988 for scatter (@etpinard edit: cartesian), pie, ternary, geo and mapbox plots.

Fixes #1456 and #902

n-riesco added 10 commits March 21, 2017 15:06
* Include the mouse event in the event data returned by `plotly_click`,
  `plotly_hover` and `plotly_unhover`.

* Keep compatibility with IE9:
  - try `new MouseEvent(type, evt)`,
  - and revert to `initMouseEvent` in case of failure.
* Test the property event in `plotly_click`, `plotly_hover` and
  `plotly_unhover`.

* Test modified clicks (e.g. ctrlKey).

* Updated the helper function click. Added a click event after mouseup,
  otherwise click events in pie plots don't get triggered.
* Test that 'plotly_hover' is triggered when `hoverinfo` is set to
  `"none"`.
* Test that 'plotly_click' doesn't emit an undefined trace.
* Use `evt.target` to determine whether the user invoked `Fx.hover` with
  a fake event.
@etpinard
Copy link
Contributor

Amazing work @n-riesco thanks very much! I'll review this at some point today.

@etpinard etpinard added status: reviewable bug something broken feature something new labels Mar 21, 2017
var customMatchers = require('../assets/custom_matchers');


function getClientPosition(selector, index) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@n-riesco Nice helper!

Would you mind making this an assets module in test/jasmine/assets/get_client_position.js?

// [x|y]px: the pixels (from top left) of the mouse location
// on the currently selected plot area
var hasUserCalledHover = ('xpx' in evt || 'ypx' in evt),
var hasUserCalledHover = !evt.target,
Copy link
Contributor

Choose a reason for hiding this comment

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

Which test(s) broke while ('xpx' in evt || 'ypx' in evt)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fx.hover('graph', evt, 'xy');

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Good to know!

hasHoverData = false;

function handleMouseOver(evt) {
evt.originalEvent = d3.event;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

function handleClick() {
gd._hoverdata = [pt];
gd._hoverdata.trace = cd.trace;
gd._hoverdata.trace = cd0.trace;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely done 🎉

@etpinard
Copy link
Contributor

@n-riesco amazing PR!

A new future spanning 5 of our 8 (did I count right?) subplot types and fixing two bugs while at it 💪

I made one blocking #1505 (comment) and one non-blocking #1505 (comment) 🚀

@etpinard
Copy link
Contributor

I'm going to start merging things for next week's v.1.26.0.

Great work @n-riesco 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken feature something new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants