Skip to content

Bug fix: applying camera up.z vector at scene init #3256

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 23 commits into from
Nov 22, 2018
Merged

Bug fix: applying camera up.z vector at scene init #3256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f82007b
fix for issue 2425
archmoj Nov 17, 2018
d8ef9eb
revised camera positions in 3 mocks
archmoj Nov 17, 2018
556268b
updated mocks and baseline images
archmoj Nov 17, 2018
eb72856
moved overwrites to layout defaults
archmoj Nov 17, 2018
789cfd9
fix syntax
archmoj Nov 17, 2018
4014c35
revised snowden mock and baseline
archmoj Nov 17, 2018
1ae12b3
revert snowden mock
archmoj Nov 17, 2018
cb6dab7
revert gl3d_surface-lighting mock to better display the old bug
archmoj Nov 18, 2018
67ea429
improved logic
archmoj Nov 18, 2018
5c3742c
minimal fix
archmoj Nov 18, 2018
3ab1b1d
changed defaults and baseline
archmoj Nov 18, 2018
5b24f51
logic to enable turntable default
archmoj Nov 19, 2018
d338058
revert gl3d layout test
archmoj Nov 19, 2018
b8da9ba
revert baselines
archmoj Nov 19, 2018
899b568
changes to gl3d layout test
archmoj Nov 19, 2018
724416b
fixes for jasmine test
archmoj Nov 19, 2018
c9eed54
fix for restyle calls
archmoj Nov 19, 2018
b65fa61
adding jasmine tests for camera init
archmoj Nov 20, 2018
d25c335
added camera tests
archmoj Nov 20, 2018
f8bf472
revised auto logic i.e. without touching layout inputs
archmoj Nov 20, 2018
6af04f2
untouch gl3d layout test
archmoj Nov 20, 2018
9ef8a70
forced graphs using deprecated cameraposition attribute to stay upwards
archmoj Nov 21, 2018
f43f944
reverted and changed baseline that uses cameraposition
archmoj Nov 21, 2018
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
Prev Previous commit
Next Next commit
fixes for jasmine test
  • Loading branch information
@archmoj
archmoj committed Nov 19, 2018
commit 724416b455da00ba847d73ca885257d31929cee7
2 changes: 1 addition & 1 deletion src/plots/gl3d/layout/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function handleGl3dDefaults(sceneLayoutIn, sceneLayoutOut, coerce, opts) {
sceneLayoutIn.dragmode = 'turntable';
}

if(!sceneLayoutIn.dragmode) {
if(sceneLayoutIn.dragmode !== undefined) {
// Because the default is now set to orbit mode
// (i.e. in order to apply camera.z.up at init time)
// we set turnable our disarable option here.
Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/tests/gl3d_plot_interact_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ describe('Test gl3d plots', function() {

it('@gl should be able to reversibly change trace type', function(done) {
var _mock = Lib.extendDeep({}, mock2);
var sceneLayout = { aspectratio: { x: 1, y: 1, z: 1 } };
var sceneLayout = { aspectratio: { x: 1, y: 1, z: 1 }, dragmode: 'turntable' };
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you need to change this? I'm fearing a breaking change here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to set turnable the default behaviour the function needs to add the dragmode attribute to the sceneLayout.

Copy link
Contributor

Choose a reason for hiding this comment

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

So without setting dragmode: 'turntable' here, it would default to 'orbit'?

What are the camera settings in this case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@etpinard notice this isn't a setting, it's an expectation. But I agree, we shouldn't be mutating the input scene, per my comment above


Plotly.plot(gd, _mock)
.then(delay(20))
Expand Down