Include gl3d scene.aspectmode changes in relayout updates #4579

archmoj · 2020-02-13T04:10:05Z

Follow up of #4578 this PR adds gl3d scene.aspectmode changes in relayout updates.

@plotly/plotly_js

etpinard · 2020-02-13T14:37:20Z

src/plots/gl3d/scene.js

@@ -219,6 +219,12 @@ proto.initializeGLPlot = function() {
        if(scene.isAspectChanged(layout)) {
            // scene updates
            update[scene.id + '.aspectratio'] = scene.glplot.getAspectratio();
+
+            if(layout[scene.id].aspectmode !== 'manual') {


How do we get back to the original aspectmode ?

When resetting the view (e.g via the modebar buttons), yes.

Good call. I think we have some tests related to that.

plotly.js/test/jasmine/tests/plot_api_test.js

Lines 394 to 411 in 4d24c36

it('sets aspectmode to manual when you provide any aspectratio', function(done) {

Plotly.plot(gd, [{x: [1, 2], y: [1, 2], z: [1, 2], type: 'scatter3d'}])

.then(function() {

expect(gd.layout.scene.aspectmode).toBe('auto');

return Plotly.relayout(gd, {'scene.aspectratio.x': 2});

})

.then(function() {

expect(gd.layout.scene.aspectmode).toBe('manual');

return Queue.undo(gd);

})

.then(function() {

expect(gd.layout.scene.aspectmode).toBe('auto');

})

.catch(failTest)

.then(done);

});

So, to be clear, once a user scrolls into a orthographic scene, the aspectmode is turned to 'manual' and the only way to get back to original aspectmode is by resetting the view?

Or we should keep that in viewInitial props?

How is it handled currently?

apsectmode added to viewInitial in 1fdc49c and tests are added in 6ae2809.

etpinard · 2020-02-14T13:44:11Z

src/plots/gl3d/scene.js

@@ -790,6 +790,9 @@ proto.plot = function(sceneData, fullLayout, layout) {
            y: fullSceneLayout.aspectratio.y,
            z: fullSceneLayout.aspectratio.z
        };
+
+        // also keep track of aspectmode here
+        scene.viewInitial.aspectmode = fullSceneLayout.aspectmode;


Shouldn't this be

if(!scene.viewInitial.aspectmode) { scene.viewInitial.aspectmode = fullSceneLayout.aspectmode }

that way we don't override viewInitia.aspectmodel on updates?

Good call.
We could. Done in c730f12.

etpinard · 2020-02-14T14:09:56Z

Thanks - 💃 !

archmoj added 2 commits February 12, 2020 22:38

include aspectmode changes in relayout updates

c50559a

improve tests for aspectmode in relayouts

20d4b3c

archmoj added bug something broken status: reviewable labels Feb 13, 2020

etpinard reviewed Feb 13, 2020

View reviewed changes

archmoj added 2 commits February 13, 2020 11:21

keep track of gl3d scene.aspectmode for modebar buttons

1fdc49c

improve aspectmode tests

6ae2809

etpinard reviewed Feb 14, 2020

View reviewed changes

record initial aspectmode safely

c730f12

archmoj merged commit 15d75db into master Feb 14, 2020

archmoj deleted the include-aspectmode-in-relayout branch February 14, 2020 14:10

This was referenced Apr 17, 2020

[Snyk] Upgrade plotly.js from 1.41.3 to 1.52.3 dunzoit/redash#4

Open

[Snyk] Upgrade plotly.js from 1.52.1 to 1.53.0 mc-ireiser/vue-plotly#1

Open

snyk-bot mentioned this pull request May 1, 2020

[Snyk] Upgrade plotly.js-basic-dist from 1.52.3 to 1.53.0 libamtrack/web#583

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include gl3d scene.aspectmode changes in relayout updates #4579

Include gl3d scene.aspectmode changes in relayout updates #4579

archmoj commented Feb 13, 2020 •

edited

Loading

etpinard Feb 13, 2020

archmoj Feb 13, 2020

etpinard Feb 13, 2020

archmoj Feb 13, 2020

archmoj Feb 13, 2020

etpinard Feb 13, 2020

archmoj Feb 13, 2020

etpinard Feb 13, 2020

archmoj Feb 13, 2020

etpinard Feb 14, 2020 •

edited

Loading

archmoj Feb 14, 2020

etpinard commented Feb 14, 2020

	it('sets aspectmode to manual when you provide any aspectratio', function(done) {
	Plotly.plot(gd, [{x: [1, 2], y: [1, 2], z: [1, 2], type: 'scatter3d'}])
	.then(function() {
	expect(gd.layout.scene.aspectmode).toBe('auto');

	return Plotly.relayout(gd, {'scene.aspectratio.x': 2});
	})
	.then(function() {
	expect(gd.layout.scene.aspectmode).toBe('manual');

	return Queue.undo(gd);
	})
	.then(function() {
	expect(gd.layout.scene.aspectmode).toBe('auto');
	})
	.catch(failTest)
	.then(done);
	});

Include gl3d scene.aspectmode changes in relayout updates #4579

Include gl3d scene.aspectmode changes in relayout updates #4579

Conversation

archmoj commented Feb 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Feb 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard commented Feb 14, 2020

archmoj commented Feb 13, 2020 •

edited

Loading

etpinard Feb 14, 2020 •

edited

Loading