-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Click anywhere
feature
#5443
Open
sleighsoft
wants to merge
13
commits into
plotly:master
Choose a base branch
from
sleighsoft:click_v1
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Click anywhere
feature
#5443
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
6a07bc9
click v1
sleighsoft 0ab1e75
fix lint
sleighsoft 603cfa1
Merge branch 'master' into click_v1
sleighsoft 279a0d1
Fix linting
sleighsoft fb506ba
Merge branch 'master' into click_v1
sleighsoft 4d221db
try to enable click anywhere in Geo mode
yiyuezhuo 461b695
fix anywhere leak problem
yiyuezhuo 0956827
Merge pull request #2 from yiyuezhuo/click_geo
sleighsoft a9c0c7b
Fix linting
sleighsoft 3506f04
Merge branch 'master' into click_v1
sleighsoft babdf89
Drop hardcoded role:info
sleighsoft 9b5c237
update plot-schema diff
sleighsoft a406275
Add geoscatter test and refactor tests
sleighsoft File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,206 @@ | ||
var Plotly = require('@lib/index'); | ||
var Lib = require('@src/lib'); | ||
var click = require('../assets/click'); | ||
|
||
var createGraphDiv = require('../assets/create_graph_div'); | ||
var destroyGraphDiv = require('../assets/destroy_graph_div'); | ||
var DBLCLICKDELAY = require('@src/plot_api/plot_config').dfltConfig.doubleClickDelay; | ||
|
||
var clickEvent; | ||
var clickedPromise; | ||
|
||
function resetEvents(gd) { | ||
clickEvent = null; | ||
|
||
gd.removeAllListeners(); | ||
|
||
clickedPromise = new Promise(function(resolve) { | ||
gd.on('plotly_click', function(data) { | ||
clickEvent = data.points[0]; | ||
resolve(); | ||
}); | ||
}); | ||
} | ||
|
||
describe('Click-to-select', function() { | ||
var mock14PtsScatter = { | ||
'in-margin': { x: 28, y: 28 }, | ||
'point-0': { x: 92, y: 102 }, | ||
'between-point-0-and-1': { x: 117, y: 110 }, | ||
'point-11': { x: 339, y: 214 }, | ||
}; | ||
var expectedEventsScatter = { | ||
'in-margin': false, | ||
'point-0': { | ||
curveNumber: 0, | ||
pointIndex: 0, | ||
pointNumber: 0, | ||
x: 0.002, | ||
y: 16.25 | ||
}, | ||
'between-point-0-and-1': { x: 0.002990379231567056, y: 14.169142943944111 }, | ||
'point-11': { | ||
curveNumber: 0, | ||
pointIndex: 11, | ||
pointNumber: 11, | ||
x: 0.125, | ||
y: 2.125 | ||
}, | ||
}; | ||
|
||
var mockPtsGeoscatter = { | ||
'start': {lat: 40.7127, lon: -74.0059}, | ||
'end': {lat: 51.5072, lon: 0.1275}, | ||
}; | ||
var mockPtsGeoscatterClick = { | ||
'in-margin': { x: 28, y: 28 }, | ||
'start': {x: 239, y: 174}, | ||
'end': {x: 426, y: 157}, | ||
'iceland': {x: 322, y: 150}, | ||
}; | ||
var expectedEventsGeoscatter = { | ||
'in-margin': false, | ||
'start': { | ||
curveNumber: 0, | ||
pointIndex: 0, | ||
pointNumber: 0, | ||
lat: 40.7127, | ||
lon: -74.0059, | ||
}, | ||
'end': { | ||
curveNumber: 0, | ||
pointIndex: 1, | ||
pointNumber: 1, | ||
lat: 51.5072, | ||
lon: 51.5072, | ||
}, | ||
'iceland': {lat: -18.666562962962963, lon: 56.66635185185185}, | ||
}; | ||
|
||
var gd; | ||
|
||
beforeEach(function() { | ||
gd = createGraphDiv(); | ||
}); | ||
|
||
afterEach(function() { | ||
resetEvents(gd); | ||
destroyGraphDiv(); | ||
}); | ||
|
||
function plotMock14Anywhere(layoutOpts) { | ||
var mock = require('@mocks/14.json'); | ||
var defaultLayoutOpts = { | ||
layout: { | ||
clickmode: 'event+anywhere', | ||
hoverdistance: 1 | ||
} | ||
}; | ||
var mockCopy = Lib.extendDeep( | ||
{}, | ||
mock, | ||
defaultLayoutOpts, | ||
{ layout: layoutOpts }); | ||
|
||
return Plotly.newPlot(gd, mockCopy.data, mockCopy.layout); | ||
} | ||
|
||
function plotMock14AnywhereSelect(layoutOpts) { | ||
var mock = require('@mocks/14.json'); | ||
var defaultLayoutOpts = { | ||
layout: { | ||
clickmode: 'select+event+anywhere', | ||
hoverdistance: 1 | ||
} | ||
}; | ||
var mockCopy = Lib.extendDeep( | ||
{}, | ||
mock, | ||
defaultLayoutOpts, | ||
{ layout: layoutOpts }); | ||
|
||
return Plotly.newPlot(gd, mockCopy.data, mockCopy.layout); | ||
} | ||
|
||
function plotGeoscatterAnywhere() { | ||
var layout = { | ||
clickmode: 'event+anywhere', | ||
hoverdistance: 1 | ||
}; | ||
var data = [{ | ||
type: 'scattergeo', | ||
lat: [ mockPtsGeoscatter.start.lat, mockPtsGeoscatter.end.lat ], | ||
lon: [ mockPtsGeoscatter.start.lon, mockPtsGeoscatter.end.lat ], | ||
mode: 'lines', | ||
line: { | ||
width: 2, | ||
color: 'blue' | ||
} | ||
}]; | ||
return Plotly.newPlot(gd, data, layout); | ||
} | ||
|
||
function isSubset(superObj, subObj) { | ||
return superObj === subObj || | ||
typeof superObj === 'object' && | ||
typeof subObj === 'object' && ( | ||
subObj.valueOf() === superObj.valueOf() || | ||
Object.keys(subObj).every(function(k) { return isSubset(superObj[k], subObj[k]); }) | ||
); | ||
} | ||
|
||
/** | ||
* Executes a click and before resets event handlers. | ||
* Returns the `clickedPromise` for convenience. | ||
*/ | ||
function _click(x, y, clickOpts) { | ||
resetEvents(gd); | ||
setTimeout(function() { | ||
click(x, y, clickOpts); | ||
}, DBLCLICKDELAY * 1.03); | ||
return clickedPromise; | ||
} | ||
|
||
function clickAndTestPoint(mockPts, expectedEvents, pointKey, clickOpts) { | ||
var x = mockPts[pointKey].x; | ||
var y = mockPts[pointKey].y; | ||
var expectedEvent = expectedEvents[pointKey]; | ||
var result = _click(x, y, clickOpts); | ||
if(expectedEvent) { | ||
result.then(function() { | ||
expect(isSubset(clickEvent, expectedEvent)).toBe(true); | ||
}); | ||
} else { | ||
expect(clickEvent).toBe(null); | ||
result = null; | ||
} | ||
return result; | ||
} | ||
|
||
it('selects point and/or coordinate when clicked - scatter - event+anywhere', function(done) { | ||
plotMock14Anywhere() | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'in-margin'); }) | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'point-0'); }) | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'between-point-0-and-1'); }) | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'point-11'); }) | ||
.then(done, done.fail); | ||
}); | ||
|
||
it('selects point and/or coordinate when clicked - scatter - select+event+anywhere', function(done) { | ||
plotMock14AnywhereSelect() | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'in-margin'); }) | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'point-0'); }) | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'between-point-0-and-1'); }) | ||
.then(function() { return clickAndTestPoint(mock14PtsScatter, expectedEventsScatter, 'point-11'); }) | ||
.then(done, done.fail); | ||
}); | ||
|
||
it('selects point and/or coordinate when clicked - geoscatter - event+anywhere', function(done) { | ||
plotGeoscatterAnywhere() | ||
.then(function() { return clickAndTestPoint(mockPtsGeoscatterClick, expectedEventsGeoscatter, 'in-margin'); }) | ||
.then(function() { return clickAndTestPoint(mockPtsGeoscatterClick, expectedEventsGeoscatter, 'start'); }) | ||
.then(function() { return clickAndTestPoint(mockPtsGeoscatterClick, expectedEventsGeoscatter, 'end'); }) | ||
.then(function() { return clickAndTestPoint(mockPtsGeoscatterClick, expectedEventsGeoscatter, 'iceland'); }) | ||
.then(done, done.fail); | ||
}); | ||
}); |
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic means that if the plot has a geo subplot, we're assuming the click was on that, even if the plot has other subplots too. But we can do a lot better than that, and there are more subplot types than just geo and 2D cartesian. And fortunately we already have a mechanism to detect this: the third arg to
click
issubplot
and it should tell us exactly which subplot the click came from.2D cartesian, ternary, and polar subplots report this correctly (eg
'x2y3'
,'ternary2'
,'polar3'
), geo doesn't but it should be easy to fix that. Then what we need to do is find the actual subplot object, and depending on its type calculate the appropriate coordinates within that particular subplot.Pie, sankey, and funnelarea also all reach this point but don't give a subplot. We could have them give the trace number I guess, but is there anything useful to report for them? Just the raw coordinates within the plot?
3D, parcoords, and parcats don't even get here, I'm happy to ignore those for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that something that you intend to do in a second round or within this PR. If it is something I should fix, then I'd like a short example ideally for each plot type as I am not familiar with most of the ones you listed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see https://rreusser.github.io/plotly-mock-viewer/#plot_types as well as https://rreusser.github.io/plotly-mock-viewer/#polar_subplots which render https://github.com/plotly/plotly.js/blob/master/test/image/mocks/plot_types.json and
https://github.com/plotly/plotly.js/blob/master/test/image/mocks/polar_subplots.json.