plotly.js
plotly.js copied to clipboard
plotly
Click `anywhere` feature
This adds a click anywhere feature as proposed in #2696
This PR is meant as a point of discussion. Feel free to leave feedback.
Demo Code:
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<script src="../plotly.js/dist/plotly.js"></script>
</head>
<body>
<div id="tester" style="width:600px;height:250px;"></div>
<script>
var TESTER = document.getElementById('tester');
Plotly.newPlot(TESTER, [{
x: [1, 2, 3, 4],
y: [4, 1, 3, 5],
mode: "lines+markers",
marker: {size: 12},
}], {
margin: { t: 0 },
clickmode: "event+anywhere"
});
TESTER.on('plotly_click', function(clickData) {
var x = clickData["points"][0]["x"];
Plotly.relayout(TESTER, {
shapes: [
{
type: "line",
x0: x,
y0: 0,
x1: x,
y1: 1,
yref: "paper",
line: {color: "RoyalBlue", width:3},
}
]
});
});
</script>
</body>
</html>
@archmoj, I have a question regarding testing. I have looked through the code and did not find how you are testing the different clickmode flags under test\jasmine\tests\click_test.js. Could you give me some guidance/help on how to implement a proper test for it, please.
@archmoj, I have a question regarding testing. I have looked through the code and did not find how you are testing the different clickmode flags under test\jasmine\tests\click_test.js. Could you give me some guidance/help on how to implement a proper test for it, please.
The "select" clickmode are tested in `select_test"
For example
https://github.com/plotly/plotly.js/blob/8d7db15492e07f656048afe3733d72e6a30dca15/test/jasmine/tests/select_test.js#L132
You could start a new test file perhaps anywhere_test to add new tests.
Also thanks for the new PR. It looks pretty close. And I don't think you need to close this one and reopen again now that you opened the PR from your click branch. If we ever wanted to merge upstream/master it should be easy to sync your repo.
Hey @archmoj, I have added a couple of test but I would like to debug them to make sure I didn't forget anything and that the expected values are set correctly. Do you have any recommendations on how I can debug a karma test? I've tried setting up the chrome debugging in vscode but wasn't successfull so far. It somehow never stops at my breakpoints even though they are at the start of the file. Any advice is very much welcome!
Hey @archmoj, I have added a couple of test but I would like to debug them to make sure I didn't forget anything and that the expected values are set correctly. Do you have any recommendations on how I can debug a karma test? I've tried setting up the chrome debugging in vscode but wasn't successfull so far. It somehow never stops at my breakpoints even though they are at the start of the file. Any advice is very much welcome!
Great news. Please see https://github.com/plotly/plotly.js/blob/master/CONTRIBUTING.md#using-the-developer-console-in-karma-to-writedebug-jasmine-tests
Do I have to push the dist/ and build/ directories as well? They are not ignored.
I have added a test case. I consider this to be finished. Let me know if there are any more changes necessary.
Do I have to push the
dist/andbuild/directories as well? They are not ignored.
No. You should not commit them.
Tests don't seem to fail due to my changes, right?
We experienced failures on the CircleCI in the last week. Could you please fetch upstream/master and merge it into your branch? That should fix the tests.
It seems that xaxis.p2d and yaxis.p2d doesn't exists in Map, gd._fullLayout.geo._subplot.xaxis.p2c() and gd._fullLayout.geo._subplot.yaxis.p2c() is an alternative to get LonLat coordinates.
A small update on timing: our team is working hard on releasing v2.0 of Plotly.js, which we anticipate will happen in early April. This PR would be a good candidate to land in the library in v2.1 or later, so with apologies for the delay, we will likely not be able to give much feedback on this PR for the next few weeks :)
It seems that
xaxis.p2dandyaxis.p2ddoesn't exists in Map,gd._fullLayout.geo._subplot.xaxis.p2c()andgd._fullLayout.geo._subplot.yaxis.p2c()is an alternative to get LonLat coordinates.
@yiyuezhuo Can you provide a sample code which you used to test Map with?
@nicolaskruchten
- When plotting a
type: "scattermapbox"plot, how do I get access to the coordinate system axes? - Are there any other chart types besides the one that is covered by
gd._fullLayout.xaxisthat I need to be aware of and support? @yiyuezhuo already mentioned geo - How can I build plotly locally so that D3.js and other required packages for above plot types work?
- For
type: "scattermapbox", I always getPlotly.d3 is undefinederrors locally
- For
@sleighsoft A sample code is given in original PR: https://github.com/sleighsoft/plotly.js/pull/2
How is the plan for this feature? When is planned to merge this into master branch?
@archmoj @alexcjohnson Besides fixing the two failed checks, what else needs done on this PR? When would this feature be merged?
Failing tests seem unrelated to my additions. From my side this feature is ready to be merged @archmoj
Failing tests seem unrelated to my additions. From my side this feature is ready to be merged @archmoj
They all passed now.
Thanks very much for the PR and the follow up. All looking good to me. Passing over to @nicolaskruchten and @alexcjohnson.
You're welcome, looking forward to being able to finally start clicking anywhere!
Thanks for your contributions here @sleighsoft! We're hard at work on v2.6.0 but I want to get this merged into 2.7.0.
How/does this interact with double-clicks, out of curiosity? Do we need to think about any special handling here @alexcjohnson ?
How/does this interact with double-clicks, out of curiosity?
I'd need to play with it to really be able to say, but I'm not worried about it: this is being inserted right where the click event would have been emitted anyway, so if there's a problem with doubleclick it's already a problem without this feature.
As far as actually reporting a usable doubleclick for users, ATM our plotly_doubleclick event always returns null event data AFAICT, so if at some point we want to do better we can handle both hoverdata and anywhere-coordinates there.
Apparently, there is also an issue with margins? https://github.com/plotly/plotly.js/issues/2696#issuecomment-915141689 Don't have time right now to check
Can someone please summarize:
- Why this PR is still open?
- Why it took until 2021 for someone to even start working on it? A click-anywhere feature seems so obvious to so many use cases. In the issues, people have been asking for it since Jan 2017 - #145.
@Pablongo24 your comment comes across as quite aggressive and demanding and its tone is not particularly welcome here.
- Because there is still unaddressed feedback in the PR.
- It's hard to answer such a question, and in some sense this is the question in open-source: there are thousands of people who in principle could have started working on it, but none of them had the right mix of time/motivation/skills to do so. If you step back a bit, it's actually more surprising that someone actually has done so: providing free labour to the rest of the world is the outlier behaviour rather than the expectation.
@Pablongo24 your comment comes across as quite aggressive and demanding and its tone is not particularly welcome here.
@nicolaskruchten not sure why it came across as aggressive. Just asked two questions out of legitimate curiosity. I understand the dynamics of working on open-source projects. Merely pointing out that in the 6 years since this issue first popped up, several other features have been added - many of them seemingly more difficult to implement (though I realize I'm not even remotely close to a JS expert, so my perception might be wrong).