trim toggle-rect to more-precise width

... the current width didn't lead to any bugs, but
    it was too large in most cases.

- make horiz. grouped / non-grouped logic more similar
-  DRY up x/y anchor logic

Author: etpinard

src/components/legend/draw.js

@@ -98,12 +98,8 @@ module.exports = function draw(gd) {
             var gs = fullLayout._size;
             var bw = opts.borderwidth;
 
-            var lx = gs.l + gs.w * opts.x;
-            if(Lib.isRightAnchor(opts)) {
-                lx -= opts._width;
-            } else if(Lib.isCenterAnchor(opts)) {
-                lx -= opts._width / 2;
-            }
+            var lx = gs.l + gs.w * opts.x - FROM_TL[getXanchor(opts)] * opts._width;
+            var ly = gs.t + gs.h * (1 - opts.y) - FROM_TL[getYanchor(opts)] * opts._effHeight;
 
 
             // Make sure the legend left and right sides are visible
@@ -119,12 +115,6 @@ module.exports = function draw(gd) {
                 legendWidth = Math.min(fullLayout.width - lx, opts._width);
             }
 
-            var ly = gs.t + gs.h * (1 - opts.y);
-            if(Lib.isBottomAnchor(opts)) {
-                ly -= opts._height;
-            } else if(Lib.isMiddleAnchor(opts)) {
-                ly -= opts._height / 2;
-            }
 
             // Make sure the legend top and bottom are visible
             // (legends with a scroll bar are not allowed to stretch beyond the extended
@@ -487,6 +477,7 @@ function computeTextDimensions(g, gd) {
 function computeLegendDimensions(gd, groups, traces) {
     var fullLayout = gd._fullLayout;
     var opts = fullLayout.legend;
+    var isVertical = helpers.isVertical(opts);
     var isGrouped = helpers.isGrouped(opts);
 
     var bw = opts.borderwidth;
@@ -495,172 +486,141 @@ function computeLegendDimensions(gd, groups, traces) {
     var itemGap = constants.itemGap;
     var endPad = 2 * (bw + itemGap);
 
-    var extraWidth = 0;
+    opts._maxWidth = fullLayout._size.w;
+
+    var toggleRectWidth = 0;
     opts._width = 0;
     opts._height = 0;
 
-    if(helpers.isVertical(opts)) {
-        if(isGrouped) {
-            groups.each(function(d, i) {
-                Drawing.setTranslate(this, 0, i * opts.tracegroupgap);
-            });
-        }
-
+    if(isVertical) {
         traces.each(function(d) {
             var h = d[0].height;
             Drawing.setTranslate(this, bw, itemGap + bw + opts._height + h / 2);
             opts._height += h;
             opts._width = Math.max(opts._width, d[0].width);
         });
 
+        toggleRectWidth = textGap + opts._width;
         opts._width += itemGap + textGap + bw2;
         opts._height += endPad;
 
         if(isGrouped) {
+            groups.each(function(d, i) {
+                Drawing.setTranslate(this, 0, i * opts.tracegroupgap);
+            });
             opts._height += (opts._lgroupsLength - 1) * opts.tracegroupgap;
         }
+    } else {
+        var maxItemWidth = 0;
+        var combinedItemWidth = 0;
+        traces.each(function(d) {
+            var w = d[0].width + textGap;
+            maxItemWidth = Math.max(maxItemWidth, w);
+            combinedItemWidth += w;
+        });
 
-        extraWidth = textGap;
-    } else if(isGrouped) {
-        var maxHeight = 0;
-        var maxWidth = 0;
-        var maxItems = 0;
-        var groupData = groups.data();
-        var i;
-
-        for(i = 0; i < groupData.length; i++) {
-            var group = groupData[i];
-            var groupWidths = group.map(function(d) { return d[0].width; });
-            var groupHeight = group.reduce(function(a, b) { return a + b[0].height; }, 0);
-            maxWidth = Math.max(maxWidth, Lib.aggNums(Math.max, null, groupWidths));
-            maxHeight = Math.max(maxHeight, groupHeight);
-            maxItems = Math.max(maxItems, group.length);
-        }
-
-        maxWidth += itemGap + textGap;
+        if(isGrouped) {
+            var groupData = groups.data();
+            var i;
 
-        var groupXOffsets = [opts._width];
-        var groupYOffsets = [];
-        var rowNum = 0;
-        for(i = 0; i < groupData.length; i++) {
-            if(fullLayout._size.w < (bw + opts._width + itemGap + maxWidth)) {
-                groupXOffsets[groupXOffsets.length - 1] = groupXOffsets[0];
-                opts._width = maxWidth;
-                rowNum++;
-            } else {
-                opts._width += maxWidth + bw;
+            var maxGroupHeight = 0;
+            for(i = 0; i < groupData.length; i++) {
+                var groupHeight = groupData[i].reduce(function(a, b) { return a + b[0].height; }, 0);
+                maxGroupHeight = Math.max(maxGroupHeight, groupHeight);
             }
 
-            var rowYOffset = (rowNum * maxHeight);
-            rowYOffset += rowNum > 0 ? opts.tracegroupgap : 0;
+            var groupXOffsets = [opts._width];
+            var groupYOffsets = [];
+            var rowNum = 0;
+            for(i = 0; i < groupData.length; i++) {
+                if((opts._width + itemGap + maxItemWidth + bw) > opts._maxWidth) {
+                    groupXOffsets[groupXOffsets.length - 1] = groupXOffsets[0];
+                    opts._width = maxItemWidth + itemGap;
+                    rowNum++;
+                } else {
+                    opts._width += maxItemWidth + itemGap;
+                }
 
-            groupYOffsets.push(rowYOffset);
-            groupXOffsets.push(opts._width);
-        }
+                groupXOffsets.push(opts._width);
+                groupYOffsets.push(rowNum * maxGroupHeight + (rowNum > 0 ? opts.tracegroupgap : 0));
+            }
 
-        groups.each(function(d, i) {
-            Drawing.setTranslate(this, groupXOffsets[i], groupYOffsets[i]);
-        });
+            groups.each(function(d, i) {
+                Drawing.setTranslate(this, groupXOffsets[i], groupYOffsets[i]);
+            });
 
-        groups.each(function() {
-            var group = d3.select(this);
-            var groupTraces = group.selectAll('g.traces');
-            var groupHeight = 0;
+            groups.each(function() {
+                var group = d3.select(this);
+                var groupTraces = group.selectAll('g.traces');
+                var groupHeight = 0;
 
-            groupTraces.each(function(d) {
-                var h = d[0].height;
-                Drawing.setTranslate(this, 0, itemGap + bw + groupHeight + h / 2);
-                groupHeight += h;
+                groupTraces.each(function(d) {
+                    var h = d[0].height;
+                    Drawing.setTranslate(this, 0, itemGap + bw + groupHeight + h / 2);
+                    groupHeight += h;
+                });
             });
-        });
 
-        var maxYLegend = groupYOffsets[groupYOffsets.length - 1] + maxHeight;
-        opts._height = maxYLegend + endPad;
+            opts._height = groupYOffsets[groupYOffsets.length - 1] + maxGroupHeight + endPad;
+            opts._width = Math.max.apply(null, groupXOffsets) + maxItemWidth + textGap + bw2;
+            toggleRectWidth = maxItemWidth;
+        } else {
+            var oneRowLegend = (combinedItemWidth + bw2 + (traces.size() - 1) * itemGap) < opts._maxWidth;
 
-        var maxOffset = Math.max.apply(null, groupXOffsets);
-        opts._width = maxOffset + maxWidth + textGap + bw2;
-    } else {
-        var rowHeight = 0;
-        var maxTraceHeight = 0;
-        var maxTraceWidth = 0;
-        var offsetX = 0;
-        var fullTracesWidth = 0;
+            var maxRowWidth = 0;
+            var maxItemHeightInRow = 0;
+            var offsetX = 0;
+            var offsetY = 0;
+            traces.each(function(d) {
+                var h = d[0].height;
+                var next = (oneRowLegend ? textGap + d[0].width : maxItemWidth) + itemGap;
+
+                if((next + bw + offsetX) > opts._maxWidth) {
+                    maxRowWidth = Math.max(maxRowWidth, offsetX);
+                    offsetX = 0;
+                    offsetY += maxItemHeightInRow;
+                    opts._height += maxItemHeightInRow;
+                    maxItemHeightInRow = 0;
+                }
 
-        // calculate largest width for traces and use for width of all legend items
-        traces.each(function(d) {
-            maxTraceWidth = Math.max(maxTraceWidth, textGap + d[0].width);
-            fullTracesWidth += textGap + d[0].width + itemGap;
-        });
+                Drawing.setTranslate(this, bw + offsetX, itemGap + bw + h / 2 + offsetY);
 
-        // check if legend fits in one row
-        var oneRowLegend = fullLayout._size.w > bw + fullTracesWidth - itemGap;
+                offsetX += next;
+                maxItemHeightInRow = Math.max(maxItemHeightInRow, h);
+            });
 
-        traces.each(function(d) {
-            var h = d[0].height;
-            var traceWidth = oneRowLegend ? textGap + d[0].width : maxTraceWidth;
-
-            if((bw + offsetX + itemGap + traceWidth) > fullLayout._size.w) {
-                offsetX = 0;
-                rowHeight += maxTraceHeight;
-                opts._height += maxTraceHeight;
-                // reset for next row
-                maxTraceHeight = 0;
+            if(oneRowLegend) {
+                opts._width = offsetX + bw2;
+                opts._height = maxItemHeightInRow + endPad;
+                toggleRectWidth = null;
+            } else {
+                opts._width = Math.max(maxRowWidth, offsetX) + bw;
+                opts._height += maxItemHeightInRow + endPad;
+                toggleRectWidth = maxItemWidth;
             }
-
-            Drawing.setTranslate(this, bw + offsetX, itemGap + bw + h / 2 + rowHeight);
-            opts._width += itemGap + traceWidth;
-
-            // keep track of tallest trace in group
-            offsetX += itemGap + traceWidth;
-            maxTraceHeight = Math.max(maxTraceHeight, h);
-        });
-
-        if(oneRowLegend) {
-            opts._height = maxTraceHeight;
-        } else {
-            opts._height += maxTraceHeight;
         }
-
-        opts._width += bw2;
-        opts._height += endPad;
     }
 
     opts._width = Math.ceil(opts._width);
     opts._height = Math.ceil(opts._height);
 
-    var isEditable = (
-        gd._context.edits.legendText ||
-        gd._context.edits.legendPosition
-    );
-
+    var edits = gd._context.edits;
+    var isEditable = edits.legendText || edits.legendPosition;
     traces.each(function(d) {
+        var traceToggle = d3.select(this).select('.legendtoggle');
         var h = d[0].height;
-        Drawing.setRect(d3.select(this).select('.legendtoggle'),
-            0,
-            -h / 2,
-            (isEditable ? 0 : opts._width) + extraWidth,
-            h
-        );
+        var w = isEditable ? textGap : (toggleRectWidth || (textGap + d[0].width));
+        if(!isVertical) w += itemGap / 2;
+        Drawing.setRect(traceToggle, 0, -h / 2, w, h);
     });
 }
 
 function expandMargin(gd) {
     var fullLayout = gd._fullLayout;
     var opts = fullLayout.legend;
+    var xanchor = getXanchor(opts);
+    var yanchor = getYanchor(opts);
 
-    var xanchor = 'left';
-    if(Lib.isRightAnchor(opts)) {
-        xanchor = 'right';
-    } else if(Lib.isCenterAnchor(opts)) {
-        xanchor = 'center';
-    }
-
-    var yanchor = 'top';
-    if(Lib.isBottomAnchor(opts)) {
-        yanchor = 'bottom';
-    } else if(Lib.isMiddleAnchor(opts)) {
-        yanchor = 'middle';
-    }
 
     Plots.autoMargin(gd, 'legend', {
         x: opts.x,
@@ -671,3 +631,15 @@ function expandMargin(gd) {
         t: opts._height * (FROM_TL[yanchor])
     });
 }
+
+function getXanchor(opts) {
+    return Lib.isRightAnchor(opts) ? 'right' :
+        Lib.isCenterAnchor(opts) ? 'center' :
+        'left';
+}
+
+function getYanchor(opts) {
+    return Lib.isBottomAnchor(opts) ? 'bottom' :
+        Lib.isMiddleAnchor(opts) ? 'middle' :
+        'top';
+}

test/jasmine/tests/legend_test.js

@@ -788,7 +788,7 @@ describe('legend restyle update', function() {
                 expect(node.attr('height')).toEqual('19');
 
                 var w = +node.attr('width');
-                expect(Math.abs(w - 160)).toBeLessThan(10);
+                expect(w).toBeWithin(113, 10);
             });
         }