RE: etienne review

- stash _titleWidth and _titleHeight once in _fullLayout.legend
- bring back test for prune unsupported global-level test and use parcoords instead of choropleth
- reuse getGradientDirection function to handle reversescale case
- reserve new lines for Lib.coerce argumnets not ternary operator
- add a jasmine test for legend.title.side relayout

Author: archmoj

src/components/legend/draw.js

@@ -64,9 +64,9 @@ module.exports = function draw(gd) {
         .style('stroke-width', opts.borderwidth + 'px');
 
 
-    var title = fullLayout.legend.title;
-    gd._fullLayout._legendTitleWidth = 0;
-    gd._fullLayout._legendTitleHeight = 0;
+    var title = opts.title;
+    opts._titleWidth = 0;
+    opts._titleHeight = 0;
     if(title.text) {
         var titleEl = Lib.ensureSingle(legend, 'text', 'legendtitletext');
         titleEl.attr('text-anchor', 'start')
@@ -383,7 +383,7 @@ function drawTexts(g, gd) {
 
     textEl.attr('text-anchor', 'start')
         .classed('user-select-none', true)
-        .call(Drawing.font, fullLayout.legend.font)
+        .call(Drawing.font, opts.font)
         .text(isEditable ? ensureLength(name, maxNameLength) : name);
 
     svgTextUtils.positionText(textEl, constants.textGap, 0);
@@ -483,10 +483,8 @@ function computeTextDimensions(g, gd) {
     var mathjaxGroup = g.select('g[class*=math-group]');
     var mathjaxNode = mathjaxGroup.node();
     var bw = gd._fullLayout.legend.borderwidth;
-    var opts = legendItem ?
-        gd._fullLayout.legend :
-        gd._fullLayout.legend.title;
-    var lineHeight = opts.font.size * LINE_SPACING;
+    var opts = gd._fullLayout.legend;
+    var lineHeight = (legendItem ? opts : opts.title).font.size * LINE_SPACING;
     var height, width;
 
     if(mathjaxNode) {
@@ -525,11 +523,11 @@ function computeTextDimensions(g, gd) {
         legendItem.height = Math.max(height, 16) + 3;
         legendItem.width = width;
     } else { // case of title
-        if(opts.side.indexOf('left') !== -1) {
-            gd._fullLayout._legendTitleWidth = width;
+        if(opts.title.side.indexOf('left') !== -1) {
+            opts._titleWidth = width;
         }
-        if(opts.side.indexOf('top') !== -1) {
-            gd._fullLayout._legendTitleHeight = height;
+        if(opts.title.side.indexOf('top') !== -1) {
+            opts._titleHeight = height;
         }
     }
 }
@@ -548,8 +546,6 @@ function computeLegendDimensions(gd, groups, traces) {
     var fullLayout = gd._fullLayout;
     var opts = fullLayout.legend;
     var gs = fullLayout._size;
-    opts._titleWidth = fullLayout._legendTitleWidth;
-    opts._titleHeight = fullLayout._legendTitleHeight;
 
     var isVertical = helpers.isVertical(opts);
     var isGrouped = helpers.isGrouped(opts);

src/components/legend/style.js

@@ -573,7 +573,7 @@ module.exports = function style(s, gd) {
                 if(s.size()) {
                     var gradientID = 'legendfill-' + trace.uid;
                     Drawing.gradient(s, gd, gradientID,
-                        reversescale ? 'horizontal' : 'horizontalreversed',
+                        getGradientDirection(reversescale),
                         colorscale, 'fill');
                 }
             };

src/plots/plots.js

@@ -1292,9 +1292,7 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac
 
         if(Registry.traceIs(traceOut, 'showLegend')) {
             Lib.coerce(traceIn, traceOut,
-                _module.attributes.showlegend ?
-                _module.attributes :
-                plots.attributes,
+                _module.attributes.showlegend ? _module.attributes : plots.attributes,
                 'showlegend'
             );
 

test/jasmine/bundle_tests/plotschema_test.js

@@ -372,6 +372,7 @@ describe('plot schema', function() {
         var traces = Plotly.PlotSchema.get().traces;
 
         expect(traces.sankey.attributes.hoverinfo.flags.length).toBe(0);
+        expect(traces.parcoords.attributes.showlegend).toBe(undefined, 'no legend attrs for parcoords (for now)');
         expect(traces.table.attributes.opacity).toBe(undefined, 'no opacity attr for table');
         expect(traces.parcoords.attributes.hoverinfo).toBe(undefined, 'no hover attrs for parcoords');
         expect(traces.scatter3d.attributes.selectedpoints).toBe(undefined, 'no selectedpoints for gl3d traces');

test/jasmine/tests/legend_test.js

@@ -781,6 +781,42 @@ describe('legend relayout update', function() {
         .catch(failTest)
         .then(done);
     });
+
+    it('should fit in graph viewport when changing legend.title.side', function(done) {
+        var fig = Lib.extendDeep({}, require('@mocks/0.json'));
+        fig.layout.legend = {
+            title: {
+                text: 'legend title'
+            }
+        };
+
+        function _assert(msg, xy, wh) {
+            return function() {
+                var fullLayout = gd._fullLayout;
+                var legend3 = d3.select('g.legend');
+                var bg3 = legend3.select('rect.bg');
+                var translate = Drawing.getTranslate(legend3);
+                var x = translate.x;
+                var y = translate.y;
+                var w = +bg3.attr('width');
+                var h = +bg3.attr('height');
+
+                expect([x, y]).toBeWithinArray(xy, 25, msg + '| legend x,y');
+                expect([w, h]).toBeWithinArray(wh, 25, msg + '| legend w,h');
+                expect(x + w <= fullLayout.width).toBe(true, msg + '| fits in x');
+                expect(y + h <= fullLayout.height).toBe(true, msg + '| fits in y');
+            };
+        }
+
+        Plotly.plot(gd, fig)
+        .then(_assert('base', [667.72, 60], [120, 83]))
+        .then(function() { return Plotly.relayout(gd, 'legend.title.side', 'left'); })
+        .then(_assert('after relayout to *left*', [607.54, 60], [180, 67]))
+        .then(function() { return Plotly.relayout(gd, 'legend.title.side', 'top'); })
+        .then(_assert('after relayout to *top*', [667.72, 60], [120, 83]))
+        .catch(failTest)
+        .then(done);
+    });
 });
 
 describe('legend orientation change:', function() {