fix #358 - restyling orientation

alexcjohnson · alexcjohnson · commit a10746653410 · 2017-09-20T10:40:25.000-04:00
diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js
@@ -1499,7 +1499,13 @@ function _restyle(gd, aobj, traces) {
                 // before we swap everything else
                 if(ai === 'orientation') {
                     param.set(newVal);
-                    if(param.get() === undoit[ai][i]) continue;
+                    // obnoxious that we need this level of coupling... but in order to
+                    // properly handle setting orientation to `null` we need to mimic
+                    // the logic inside Bars.supplyDefaults for default orientation
+                    var defaultOrientation = (cont.x && !cont.y) ? 'h' : 'v';
+                    if((param.get() || defaultOrientation) === contFull.orientation) {
+                        continue;
+                    }
                 }
                 // orientationaxes has no value,
                 // it flips everything and the axes
@@ -1508,6 +1514,7 @@ function _restyle(gd, aobj, traces) {
                         {v: 'h', h: 'v'}[contFull.orientation];
                 }
                 helpers.swapXYData(cont);
+                flags.calc = flags.clearAxisTypes = true;
             }
             else if(Plots.dataArrayContainers.indexOf(param.parts[0]) !== -1) {
                 // TODO: use manageArrays.applyContainerArrayChanges here too
diff --git a/test/jasmine/assets/check_ticks.js b/test/jasmine/assets/check_ticks.js
@@ -0,0 +1,11 @@
+'use strict';
+
+var d3 = require('d3');
+
+module.exports = function checkTicks(axLetter, vals, msg) {
+    var selection = d3.selectAll('.' + axLetter + 'tick text');
+    expect(selection.size()).toBe(vals.length);
+    selection.each(function(d, i) {
+        expect(d3.select(this).text()).toBe(vals[i], msg + ': ' + i);
+    });
+};
diff --git a/test/jasmine/assets/negate_if.js b/test/jasmine/assets/negate_if.js
@@ -0,0 +1,16 @@
+'use strict';
+
+/**
+ * Conditionally negate an expectation:
+ *
+ *   negateIf(expect(x), xIsSomethingElse).toBe(0);
+ *
+ * is equivalent to:
+ *
+ *   if(xIsSomethingElse) expect(x).not.toBe(0);
+ *   else expect(x).toBe(0);
+ */
+module.exports = function negateIf(expectation, negate) {
+    if(negate) return expectation.not;
+    return expectation;
+};
diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js
@@ -11,6 +11,10 @@ var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var fail = require('../assets/fail_test');
 var customMatchers = require('../assets/custom_matchers');
+var checkTicks = require('../assets/check_ticks');
+var negateIf = require('../assets/negate_if');
+
+var d3 = require('d3');
 
 describe('Bar.supplyDefaults', function() {
     'use strict';
@@ -762,10 +766,16 @@ describe('Bar.setPositions', function() {
 describe('A bar plot', function() {
     'use strict';
 
+    var gd;
+
     beforeAll(function() {
         jasmine.addMatchers(customMatchers);
     });
 
+    beforeEach(function() {
+        gd = createGraphDiv();
+    });
+
     afterEach(destroyGraphDiv);
 
     function getAllTraceNodes(node) {
@@ -830,15 +840,13 @@ describe('A bar plot', function() {
     }
 
     it('should show bar texts (inside case)', function(done) {
-        var gd = createGraphDiv(),
-            data = [{
-                y: [10, 20, 30],
-                type: 'bar',
-                text: ['1', 'Very very very very very long bar text'],
-                textposition: 'inside',
-            }],
-            layout = {
-            };
+        var data = [{
+            y: [10, 20, 30],
+            type: 'bar',
+            text: ['1', 'Very very very very very long bar text'],
+            textposition: 'inside',
+        }];
+        var layout = {};
 
         Plotly.plot(gd, data, layout).then(function() {
             var traceNodes = getAllTraceNodes(gd),
@@ -862,16 +870,15 @@ describe('A bar plot', function() {
     });
 
     it('should show bar texts (outside case)', function(done) {
-        var gd = createGraphDiv(),
-            data = [{
-                y: [10, -20, 30],
-                type: 'bar',
-                text: ['1', 'Very very very very very long bar text'],
-                textposition: 'outside',
-            }],
-            layout = {
-                barmode: 'relative'
-            };
+        var data = [{
+            y: [10, -20, 30],
+            type: 'bar',
+            text: ['1', 'Very very very very very long bar text'],
+            textposition: 'outside',
+        }];
+        var layout = {
+            barmode: 'relative'
+        };
 
         Plotly.plot(gd, data, layout).then(function() {
             var traceNodes = getAllTraceNodes(gd),
@@ -896,15 +903,13 @@ describe('A bar plot', function() {
     });
 
     it('should show bar texts (horizontal case)', function(done) {
-        var gd = createGraphDiv(),
-            data = [{
-                x: [10, -20, 30],
-                type: 'bar',
-                text: ['Very very very very very long bar text', -20],
-                textposition: 'outside',
-            }],
-            layout = {
-            };
+        var data = [{
+            x: [10, -20, 30],
+            type: 'bar',
+            text: ['Very very very very very long bar text', -20],
+            textposition: 'outside',
+        }];
+        var layout = {};
 
         Plotly.plot(gd, data, layout).then(function() {
             var traceNodes = getAllTraceNodes(gd),
@@ -929,17 +934,16 @@ describe('A bar plot', function() {
     });
 
     it('should show bar texts (barnorm case)', function(done) {
-        var gd = createGraphDiv(),
-            data = [{
-                x: [100, -100, 100],
-                type: 'bar',
-                text: [100, -100, 100],
-                textposition: 'outside',
-            }],
-            layout = {
-                barmode: 'relative',
-                barnorm: 'percent'
-            };
+        var data = [{
+            x: [100, -100, 100],
+            type: 'bar',
+            text: [100, -100, 100],
+            textposition: 'outside',
+        }];
+        var layout = {
+            barmode: 'relative',
+            barnorm: 'percent'
+        };
 
         Plotly.plot(gd, data, layout).then(function() {
             var traceNodes = getAllTraceNodes(gd),
@@ -964,8 +968,7 @@ describe('A bar plot', function() {
     });
 
     it('should be able to restyle', function(done) {
-        var gd = createGraphDiv(),
-            mock = Lib.extendDeep({}, require('@mocks/bar_attrs_relative'));
+        var mock = Lib.extendDeep({}, require('@mocks/bar_attrs_relative'));
 
         Plotly.plot(gd, mock.data, mock.layout).then(function() {
             var cd = gd.calcdata;
@@ -1114,27 +1117,26 @@ describe('A bar plot', function() {
     });
 
     it('should coerce text-related attributes', function(done) {
-        var gd = createGraphDiv(),
-            data = [{
-                y: [10, 20, 30, 40],
-                type: 'bar',
-                text: ['T1P1', 'T1P2', 13, 14],
-                textposition: ['inside', 'outside', 'auto', 'BADVALUE'],
-                textfont: {
-                    family: ['"comic sans"'],
-                    color: ['red', 'green'],
-                },
-                insidetextfont: {
-                    size: [8, 12, 16],
-                    color: ['black'],
-                },
-                outsidetextfont: {
-                    size: [null, 24, 32]
-                }
-            }],
-            layout = {
-                font: {family: 'arial', color: 'blue', size: 13}
-            };
+        var data = [{
+            y: [10, 20, 30, 40],
+            type: 'bar',
+            text: ['T1P1', 'T1P2', 13, 14],
+            textposition: ['inside', 'outside', 'auto', 'BADVALUE'],
+            textfont: {
+                family: ['"comic sans"'],
+                color: ['red', 'green'],
+            },
+            insidetextfont: {
+                size: [8, 12, 16],
+                color: ['black'],
+            },
+            outsidetextfont: {
+                size: [null, 24, 32]
+            }
+        }];
+        var layout = {
+            font: {family: 'arial', color: 'blue', size: 13}
+        };
 
         var expected = {
             y: [10, 20, 30, 40],
@@ -1194,6 +1196,72 @@ describe('A bar plot', function() {
         .catch(fail)
         .then(done);
     });
+
+    it('can change orientation and correctly sets axis types', function(done) {
+        function checkBarsMatch(dims, msg) {
+            var bars = d3.selectAll('.bars .point');
+            var bbox1 = bars.node().getBoundingClientRect();
+            bars.each(function(d, i) {
+                if(!i) return;
+                var bbox = this.getBoundingClientRect();
+                ['left', 'right', 'top', 'bottom', 'width', 'height'].forEach(function(dim) {
+                    negateIf(expect(bbox[dim]), dims.indexOf(dim) === -1)
+                        .toBeWithin(bbox1[dim], 0.1, msg + ' (' + i + '): ' + dim);
+                });
+            });
+        }
+
+        Plotly.newPlot(gd, [{
+            x: ['a', 'b', 'c'],
+            y: [1, 2, 3],
+            type: 'bar'
+        }], {
+            width: 400, height: 400
+        })
+        .then(function() {
+            checkTicks('x', ['a', 'b', 'c'], 'initial x');
+            checkTicks('y', ['0', '0.5', '1', '1.5', '2', '2.5', '3'], 'initial y');
+
+            checkBarsMatch(['bottom', 'width'], 'initial');
+
+            // turn implicit "v" into explicit "v" - a noop but specifically
+            // for orientation this was broken at one point...
+            return Plotly.restyle(gd, {orientation: 'v'});
+        })
+        .then(function() {
+            checkTicks('x', ['a', 'b', 'c'], 'explicit v x');
+            checkTicks('y', ['0', '0.5', '1', '1.5', '2', '2.5', '3'], 'explicit v y');
+
+            checkBarsMatch(['bottom', 'width'], 'explicit v');
+
+            // back to implicit v
+            return Plotly.restyle(gd, {orientation: null});
+        })
+        .then(function() {
+            checkTicks('x', ['a', 'b', 'c'], 'implicit v x');
+            checkTicks('y', ['0', '0.5', '1', '1.5', '2', '2.5', '3'], 'implicit v y');
+
+            checkBarsMatch(['bottom', 'width'], 'implicit v');
+
+            return Plotly.restyle(gd, {orientation: 'h'});
+        })
+        .then(function() {
+            checkTicks('x', ['0', '1', '2', '3'], 'h x');
+            checkTicks('y', ['a', 'b', 'c'], 'h y');
+
+            checkBarsMatch(['left', 'height'], 'initial');
+
+            return Plotly.restyle(gd, {orientation: 'v'});
+        })
+        .then(function() {
+            checkTicks('x', ['a', 'b', 'c'], 'final x');
+            checkTicks('y', ['0', '0.5', '1', '1.5', '2', '2.5', '3'], 'final y');
+
+            checkBarsMatch(['bottom', 'width'], 'final');
+        })
+        .catch(fail)
+        .then(done);
+    });
 });
 
 describe('bar hover', function() {
diff --git a/test/jasmine/tests/contour_test.js b/test/jasmine/tests/contour_test.js
@@ -10,6 +10,7 @@ var fail = require('../assets/fail_test');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var customMatchers = require('../assets/custom_matchers');
+var checkTicks = require('../assets/check_ticks');
 
 var d3 = require('d3');
 
@@ -359,14 +360,6 @@ describe('contour edits', function() {
     });
     afterEach(destroyGraphDiv);
 
-    function checkTicks(axLetter, vals, msg) {
-        var selection = d3.selectAll('.' + axLetter + 'tick text');
-        expect(selection.size()).toBe(vals.length);
-        selection.each(function(d, i) {
-            expect(d3.select(this).text()).toBe(vals[i], msg + ': ' + i);
-        });
-    }
-
     it('can restyle x/y to different types', function(done) {
         Plotly.newPlot(gd, [{
             type: 'contour',
diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js
@@ -16,6 +16,8 @@ var customMatchers = require('../assets/custom_matchers');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var fail = require('../assets/fail_test');
+var checkTicks = require('../assets/check_ticks');
+var negateIf = require('../assets/negate_if');
 
 
 describe('Test plot api', function() {
@@ -907,11 +909,6 @@ describe('Test plot api', function() {
                 .then(done);
         });
 
-        function negateIf(condition, negate) {
-            if(negate) return condition.not;
-            return condition;
-        }
-
         it('turns off zauto when you edit zmin or zmax', function(done) {
             var zmin0 = 2;
             var zmax1 = 10;
@@ -1184,31 +1181,23 @@ describe('Test plot api', function() {
         });
 
         it('updates box position and axis type when it falls back to name', function(done) {
-            function checkXTicks(vals, msg) {
-                var selection = d3.selectAll('.xtick');
-                expect(selection.size()).toBe(vals.length);
-                selection.each(function(d, i) {
-                    expect(d3.select(this).text()).toBe(vals[i], msg + ': ' + i);
-                });
-            }
-
             Plotly.newPlot(gd, [{name: 'A', y: [1, 2, 3, 4, 5], type: 'box'}],
                 {width: 400, height: 400, xaxis: {nticks: 3}}
             )
             .then(function() {
-                checkXTicks(['A'], 'initial');
+                checkTicks('x', ['A'], 'initial');
                 expect(gd._fullLayout.xaxis.type).toBe('category');
 
                 return Plotly.restyle(gd, {name: 'B'});
             })
             .then(function() {
-                checkXTicks(['B'], 'changed category');
+                checkTicks('x', ['B'], 'changed category');
                 expect(gd._fullLayout.xaxis.type).toBe('category');
 
                 return Plotly.restyle(gd, {x0: 12.3});
             })
             .then(function() {
-                checkXTicks(['12', '12.5'], 'switched to numeric');
+                checkTicks('x', ['12', '12.5'], 'switched to numeric');
                 expect(gd._fullLayout.xaxis.type).toBe('linear');
             })
             .catch(fail)
