Add mouse event to plotly_click, plotly_hover and plotly_unhover
#1505
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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.
|
Amazing work @n-riesco thanks very much! I'll review this at some point today. |
etpinard
reviewed
Mar 21, 2017
test/jasmine/tests/ternary_test.js
Outdated
| @@ -12,6 +12,16 @@ var doubleClick = require('../assets/double_click'); | |||
| var customMatchers = require('../assets/custom_matchers'); | |||
|
|
|||
|
|
|||
| function getClientPosition(selector, index) { | |||
There was a problem hiding this comment.
@n-riesco Nice helper!
Would you mind making this an assets module in test/jasmine/assets/get_client_position.js?
etpinard
reviewed
Mar 21, 2017
| @@ -414,7 +414,7 @@ function hover(gd, evt, subplot) { | |||
|
|
|||
| // [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, | |||
There was a problem hiding this comment.
Which test(s) broke while ('xpx' in evt || 'ypx' in evt)?
There was a problem hiding this comment.
etpinard
reviewed
Mar 21, 2017
| @@ -86,6 +86,8 @@ module.exports = function plot(gd, cdpie) { | |||
| hasHoverData = false; | |||
|
|
|||
| function handleMouseOver(evt) { | |||
| evt.originalEvent = d3.event; | |||
etpinard
reviewed
Mar 21, 2017
src/traces/pie/plot.js
Outdated
| @@ -149,7 +149,7 @@ module.exports = function plot(gd, cdpie) { | |||
|
|
|||
| function handleClick() { | |||
| gd._hoverdata = [pt]; | |||
| gd._hoverdata.trace = cd.trace; | |||
| gd._hoverdata.trace = cd0.trace; | |||
|
@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) 🚀 |
This was referenced Mar 29, 2017
|
I'm going to start merging things for next week's Great work @n-riesco 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this PR, listeners to
plotly_click,plotly_hoverandplotly_unhoverreceive the original mouse event (that triggered the event) indata.event.Implements #988 for scatter (@etpinard edit: cartesian), pie, ternary, geo and mapbox plots.
Fixes #1456 and #902