Skip to content

Emit a plotly_relayout' event upon mouseup, wheel #466

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.

Sign up for GitHub

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

Merged
merged 19 commits into from
May 6, 2016
Merged

Emit a plotly_relayout' event upon mouseup, wheel #466

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a7177f2
#30a emit an event upon manipulation
monfera Apr 22, 2016
1b19b3a
#30a removing preexisting lint exemption directives and fixing former…
monfera Apr 22, 2016
34d15aa
#30a removing preexisting lint exemption directives and fixing former…
monfera Apr 22, 2016
3d8fe81
#30a covering 2D WebGL plot with the plotly_relayout event on pan / zoom
monfera Apr 28, 2016
a10caec
#30a test case for mouse drag and wheel on gl3d plots
monfera Apr 29, 2016
604d739
#30a proper grouping by interaction type
monfera Apr 29, 2016
ee8ff65
#30a test case for mouse drag on gl2d plots
monfera Apr 29, 2016
da01879
#30a reifying plotly_relayout callback count, as well as layout data …
monfera May 2, 2016
48682a4
#30a adding graphDiv to Scene2d similarly to Scene / thanks @etpinard
monfera May 2, 2016
63ec931
#30a bumping jasmine to 2.4 to enjoy its new facilities such as spy.t…
monfera May 2, 2016
19c478d
#30a panning test on WebGL canvas; adding the DIV update for scene2d
monfera May 3, 2016
e467c88
#30a saving the camera position on the DIV layout when the projection…
monfera May 3, 2016
b212086
#30a PR feedback: not expose scene id and use axis names. Plus: test …
monfera May 3, 2016
e8aa9cb
#30a PR feedback: remove `delay`
monfera May 3, 2016
e02a3f6
#30a adding tests for the 3d callback and DIV layout camera data update
monfera May 3, 2016
4975417
#30a PR feedback: commenting and removing disused binding
monfera May 4, 2016
7c23e2e
#30a PR feedback: adding test case with partially set camera and solv…
monfera May 6, 2016
7eef2e1
#30a removing new test case from the test file that's currently ignor…
monfera May 6, 2016
2a13e92
#30a excluding gl_plot_interact_basic_test.js from the CI run. Benefi…
monfera May 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plots/gl3d/scene.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ function initializeGLPlot(scene, fullLayout, canvas, gl) {
var relayoutCallback = function(scene) {
var update = {};
update[scene.id] = getLayoutCamera(scene.camera);
scene.saveCamera(scene.graphDiv.layout);
Copy link
Contributor

@etpinard etpinard May 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'm ok with this here.

BUT we should add a comment in the resetCameraLastSave mode bar button handler.

That handler looks in the un-updated fullLayout.scene.camera object to reset the camera to the last saved position.

Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks if scene.graphDiv.layout is not fully set.

For example, using the gl3d_errorbars_zx mock - which doesn't completly set scene.camera - lead on every mouse up, or scroll to:

image

The saveCamera method needs to be patched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard thanks for this find! I'll add a test case that covers this circumstance and resolve it. It's late and I'm planning to look at it early morning, but somewhat surprised as the 3d test I already put in, e.g. gl3d_scatter3d-connectgaps.json doesn't set the camera yet it didn't throw on the interaction test, so I'll start from your example above.

Copy link
Contributor

Choose a reason for hiding this comment

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

no rush, the next release will be published next Tuesday or Wednesday.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard may I ask you to check if this single-line modification in saveCamera is all it takes - I infer from the code that the sameness check needs to be prepared for the possibility of a partially set camera.

Also, I refactored the test case under which I put a new test for the partially set camera, and I hope you consider it an improvement over my original test case - it's more promise-oriented and explicitly passes around values rather than relying on globals.

If it's OK with you, I'd try to put some of the test cases into a separate file, because this test file is excluded from the CI run due to unpredictability of some of the tests. I'm uncomfortable with the notion that an ever increasing number of tests is excluded so I'd try to loop some of them in.

Copy link
Contributor

Choose a reason for hiding this comment

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

may I ask you to check if this single-line modification in saveCamera is all it takes

Yes. This is all we need. Thanks!

Also, I refactored the test case under which I put a new test for the partially set camera, and I hope you consider it an improvement over my original test case - it's more promise-oriented and explicitly passes around values rather than relying on globals

Sure. Go for it!

If it's OK with you, I'd try to put some of the test cases into a separate file, because this test file is excluded from the CI run due to unpredictability of some of the tests. I'm uncomfortable with the notion that an ever increasing number of tests is excluded so I'd try to loop some of them in.

+1

scene.graphDiv.emit('plotly_relayout', update);
};

Expand Down