From 5ef69b38e3759edba66b2d21fa8508221a0b0dff Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sat, 9 Apr 2016 00:08:32 +0100 Subject: [PATCH 1/4] Remove unnecessary code in repositionLegend() --- src/components/legend/draw.js | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index c7b47a2ce1f..1f7f0987154 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -368,7 +368,6 @@ function drawTexts(context, gd, d, i, traces) { function repositionLegend(gd, traces) { var fullLayout = gd._fullLayout, - gs = fullLayout._size, opts = fullLayout.legend, borderwidth = opts.borderwidth; @@ -432,39 +431,27 @@ function repositionLegend(gd, traces) { .attr('width', (gd._context.editable ? 0 : opts.width) + 40); // now position the legend. for both x,y the positions are recorded as - // fractions of the plot area (left, bottom = 0,0). Outside the plot - // area is allowed but position will be clipped to the page. - // values <1/3 align the low side at that fraction, 1/3-2/3 align the - // center at that fraction, >2/3 align the right at that fraction - - var lx = gs.l + gs.w * opts.x, - ly = gs.t + gs.h * (1-opts.y); + // fractions of the plot area (left, bottom = 0,0). var xanchor = 'left'; if(anchorUtils.isRightAnchor(opts)) { - lx -= opts.width; xanchor = 'right'; } if(anchorUtils.isCenterAnchor(opts)) { - lx -= opts.width / 2; xanchor = 'center'; } var yanchor = 'top'; if(anchorUtils.isBottomAnchor(opts)) { - ly -= opts.height; yanchor = 'bottom'; } if(anchorUtils.isMiddleAnchor(opts)) { - ly -= opts.height / 2; yanchor = 'middle'; } // make sure we're only getting full pixels opts.width = Math.ceil(opts.width); opts.height = Math.ceil(opts.height); - lx = Math.round(lx); - ly = Math.round(ly); // lastly check if the margin auto-expand has changed Plots.autoMargin(gd, 'legend', { From c68ed58d95b3949a90c8efa00ee413434d271976 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Mon, 11 Apr 2016 10:00:23 +0100 Subject: [PATCH 2/4] Legend.draw: Add missing `select('rect')` --- src/components/legend/draw.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 1f7f0987154..3ca247f63b7 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -227,7 +227,7 @@ module.exports = function draw(gd) { width: opts.width - 2 * opts.borderwidth + constants.scrollBarWidth }); - clipPath.attr({ + clipPath.select('rect').attr({ width: opts.width + constants.scrollBarWidth }); From 680dcd31d9e6bf58d996abee085dd025beaacbd1 Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Sat, 9 Apr 2016 12:06:17 +0100 Subject: [PATCH 3/4] Fix invisible legends with negative y * Ensure that when the margins are expended to allocate the legend, there is enough space to clear the layout margins. Fixes #384 --- src/components/legend/draw.js | 50 ++++++++++++++++++++++++----------- src/plots/plots.js | 30 +++++++++++++++++++++ 2 files changed, 64 insertions(+), 16 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 3ca247f63b7..87214c8ae4c 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -184,27 +184,42 @@ module.exports = function draw(gd) { if(anchorUtils.isRightAnchor(opts)) { lx -= opts.width; } - if(anchorUtils.isCenterAnchor(opts)) { + else if(anchorUtils.isCenterAnchor(opts)) { lx -= opts.width / 2; } if(anchorUtils.isBottomAnchor(opts)) { ly -= opts.height; } - if(anchorUtils.isMiddleAnchor(opts)) { + else if(anchorUtils.isMiddleAnchor(opts)) { ly -= opts.height / 2; } + lx = Math.round(lx); + ly = Math.round(ly); + + // Make sure the legend top is below the top margin + if(ly < fullLayout.margin.t) ly = fullLayout.margin.t; + + var scrollHeightMax = fullLayout.height - fullLayout.margin.b - ly; + var scrollHeight = Math.min(scrollHeightMax, opts.height); + + if(scrollHeight <= 2 * opts.borderwidth) { + console.error('Legend.draw: insufficient space to draw legend'); + legend.remove(); + return; + } + // Deal with scrolling - var plotHeight = fullLayout.height - fullLayout.margin.b, - scrollheight = Math.min(plotHeight - ly, opts.height), - scrollPosition = scrollBox.attr('data-scroll') ? scrollBox.attr('data-scroll') : 0; + var scrollPosition = scrollBox.attr('data-scroll') ? + scrollBox.attr('data-scroll') : + 0; scrollBox.attr('transform', 'translate(0, ' + scrollPosition + ')'); bg.attr({ width: opts.width - 2 * opts.borderwidth, - height: scrollheight - 2 * opts.borderwidth, + height: scrollHeight - 2 * opts.borderwidth, x: opts.borderwidth, y: opts.borderwidth }); @@ -213,7 +228,7 @@ module.exports = function draw(gd) { clipPath.select('rect').attr({ width: opts.width, - height: scrollheight, + height: scrollHeight, x: 0, y: 0 }); @@ -221,7 +236,7 @@ module.exports = function draw(gd) { legend.call(Drawing.setClipUrl, clipId); // If scrollbar should be shown. - if(opts.height - scrollheight > 0 && !gd._context.staticPlot) { + if(opts.height - scrollHeight > 0 && !gd._context.staticPlot) { bg.attr({ width: opts.width - 2 * opts.borderwidth + constants.scrollBarWidth @@ -243,21 +258,21 @@ module.exports = function draw(gd) { scrollBox.attr('data-scroll',0); } - scrollHandler(0,scrollheight); + scrollHandler(0,scrollHeight); legend.on('wheel',null); legend.on('wheel', function() { var e = d3.event; e.preventDefault(); - scrollHandler(e.deltaY / 20, scrollheight); + scrollHandler(e.deltaY / 20, scrollHeight); }); scrollBar.on('.drag',null); scrollBox.on('.drag',null); var drag = d3.behavior.drag() .on('drag', function() { - scrollHandler(d3.event.dy, scrollheight); + scrollHandler(d3.event.dy, scrollHeight); }); scrollBar.call(drag); @@ -266,12 +281,12 @@ module.exports = function draw(gd) { } - function scrollHandler(delta, scrollheight) { + function scrollHandler(delta, scrollHeight) { - var scrollBarTrack = scrollheight - constants.scrollBarHeight - 2 * constants.scrollBarMargin, + var scrollBarTrack = scrollHeight - constants.scrollBarHeight - 2 * constants.scrollBarMargin, translateY = scrollBox.attr('data-scroll'), - scrollBoxY = Lib.constrain(translateY - delta, scrollheight-opts.height, 0), - scrollBarY = -scrollBoxY / (opts.height - scrollheight) * scrollBarTrack + constants.scrollBarMargin; + scrollBoxY = Lib.constrain(translateY - delta, scrollHeight-opts.height, 0), + scrollBarY = -scrollBoxY / (opts.height - scrollHeight) * scrollBarTrack + constants.scrollBarMargin; scrollBox.attr('data-scroll', scrollBoxY); scrollBox.attr('transform', 'translate(0, ' + scrollBoxY + ')'); @@ -454,7 +469,10 @@ function repositionLegend(gd, traces) { opts.height = Math.ceil(opts.height); // lastly check if the margin auto-expand has changed - Plots.autoMargin(gd, 'legend', { + // (using Plots.autoMarginVertical to ensure the requested margins are + // padded with the layout vertical margins to ensure the legend doesn't + // exceed the plot area) + Plots.autoMarginVertical(gd, 'legend', { x: opts.x, y: opts.y, l: opts.width * ({right: 1, center: 0.5}[xanchor] || 0), diff --git a/src/plots/plots.js b/src/plots/plots.js index d719ffc6c81..b10184f9e84 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -875,6 +875,36 @@ plots.autoMargin = function(gd, id, o) { } }; +// Similar to plots.autoMargin, except that it pads the request with the layout +// margins, so that an object can be drawn without reaching the layout vertical +// margins. +plots.autoMarginVertical = function(gd, id, o) { + var fullLayout = gd._fullLayout; + + if(!fullLayout._pushmargin) fullLayout._pushmargin = {}; + + if(fullLayout.margin.autoexpand !== false) { + if(!o) delete fullLayout._pushmargin[id]; + else { + var pad = o.pad === undefined ? 12 : o.pad; + + // if the item is too big, just give it enough automargin to + // make sure you can still grab it and bring it back + if(o.l+o.r > fullLayout.width*0.5) o.l = o.r = 0; + if(o.b+o.t > fullLayout.height*0.5) o.b = o.t = 0; + + fullLayout._pushmargin[id] = { + l: {val: o.x, size: o.l + pad}, + r: {val: o.x, size: o.r + pad}, + b: {val: o.y, size: o.b + fullLayout.margin.b}, + t: {val: o.y, size: o.t + fullLayout.margin.t} + }; + } + + if(!gd._replotting) plots.doAutoMargin(gd); + } +}; + plots.doAutoMargin = function(gd) { var fullLayout = gd._fullLayout; if(!fullLayout._size) fullLayout._size = {}; From ca23f6c0496c388bbe0a437b74e061c081c3b66f Mon Sep 17 00:00:00 2001 From: Nicolas Riesco Date: Mon, 11 Apr 2016 12:00:16 +0100 Subject: [PATCH 4/4] Do not round off the legend location * Commented out the statements to round off the legend location, in order to reproduce the baseline images. * There are still 4 image tests that fail: 27, 5, autorange-tozero-rangemode, and range-selector-style. All these mocks locate the legend above the axis (i.e. y > 1). --- src/components/legend/draw.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 87214c8ae4c..1eb61a32d3f 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -195,8 +195,8 @@ module.exports = function draw(gd) { ly -= opts.height / 2; } - lx = Math.round(lx); - ly = Math.round(ly); + //lx = Math.round(lx); + //ly = Math.round(ly); // Make sure the legend top is below the top margin if(ly < fullLayout.margin.t) ly = fullLayout.margin.t;