Skip to content

Fix issue #384 (invisible legends when y is negative) #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 18, 2016
Merged

Fix issue #384 (invisible legends when y is negative) #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5ef69b3
Remove unnecessary code in repositionLegend()
n-riesco Apr 8, 2016
c68ed58
Legend.draw: Add missing `select('rect')`
n-riesco Apr 11, 2016
91c7757
Fix invisible legends with negative y
n-riesco Apr 9, 2016
cedabd3
Do not round off the legend location
n-riesco Apr 11, 2016
7f71b53
Fix computation of lowest possible legend position
n-riesco Apr 12, 2016
b747224
Fix position of legend scrollbar
n-riesco Apr 12, 2016
8baac47
Don't expand vertical margin if legend doesn't fit
n-riesco Apr 13, 2016
c96581c
Improve drag and wheel of legend scrollbars
n-riesco Apr 13, 2016
a05080d
Update legend mouse wheel test
n-riesco Apr 13, 2016
e1effce
Honour the scrollbar margin
n-riesco Apr 14, 2016
c581989
Add mock with a legend located at y -1
n-riesco Apr 14, 2016
d5e9014
Remove commented-out lines from Legend.draw()
n-riesco Apr 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 109 additions & 60 deletions src/components/legend/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,18 @@ module.exports = function draw(gd) {
});

// Position and size the legend
repositionLegend(gd, traces);
var lyMin = 0,
lyMax = fullLayout.height;

computeLegendDimensions(gd, traces);

if(opts.height > lyMax) {
// If the legend doesn't fit in the plot area,
// do not expand the vertical margins.
expandHorizontalMargin(gd);
} else {
expandMargin(gd);
}

// Scroll section must be executed after repositionLegend.
// It requires the legend width, height, x and y to position the scrollbox
Expand All @@ -184,27 +195,41 @@ 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;
}

// Make sure the legend top and bottom are visible
// (legends with a scroll bar are not allowed to stretch beyond the extended
// margins)
var legendHeight = opts.height,
legendHeightMax = gs.h;

if(legendHeight > legendHeightMax) {
ly = gs.t;
legendHeight = legendHeightMax;
}
else {
if(ly > lyMax) ly = lyMax - legendHeight;
if(ly < lyMin) ly = lyMin;
legendHeight = Math.min(lyMax - ly, opts.height);
}

// 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') || 0;

scrollBox.attr('transform', 'translate(0, ' + scrollPosition + ')');

bg.attr({
width: opts.width - 2 * opts.borderwidth,
height: scrollheight - 2 * opts.borderwidth,
height: legendHeight - 2 * opts.borderwidth,
x: opts.borderwidth,
y: opts.borderwidth
});
Expand All @@ -213,71 +238,81 @@ module.exports = function draw(gd) {

clipPath.select('rect').attr({
width: opts.width,
height: scrollheight,
height: legendHeight,
x: 0,
y: 0
});

legend.call(Drawing.setClipUrl, clipId);

// If scrollbar should be shown.
if(opts.height - scrollheight > 0 && !gd._context.staticPlot) {
if(opts.height - legendHeight > 0 && !gd._context.staticPlot) {

// increase the background and clip-path width
// by the scrollbar width and margin
bg.attr({
width: opts.width - 2 * opts.borderwidth + constants.scrollBarWidth
width: opts.width -
2 * opts.borderwidth +
constants.scrollBarWidth +
constants.scrollBarMargin
});

clipPath.attr({
width: opts.width + constants.scrollBarWidth
clipPath.select('rect').attr({
width: opts.width +
constants.scrollBarWidth +
constants.scrollBarMargin
});

if(gd.firstRender) {
// Move scrollbar to starting position
scrollBar.call(
Drawing.setRect,
opts.width - (constants.scrollBarWidth + constants.scrollBarMargin),
constants.scrollBarMargin,
constants.scrollBarWidth,
constants.scrollBarHeight
);
scrollBox.attr('data-scroll',0);
scrollHandler(constants.scrollBarMargin, 0);
}

scrollHandler(0,scrollheight);
var scrollBarYMax = legendHeight -
constants.scrollBarHeight -
2 * constants.scrollBarMargin,
scrollBoxYMax = opts.height - legendHeight,
scrollBarY = constants.scrollBarMargin,
scrollBoxY = 0;

legend.on('wheel',null);
scrollHandler(scrollBarY, scrollBoxY);

legend.on('wheel',null);
legend.on('wheel', function() {
var e = d3.event;
e.preventDefault();
scrollHandler(e.deltaY / 20, scrollheight);
scrollBoxY = Lib.constrain(
scrollBox.attr('data-scroll') -
d3.event.deltaY / scrollBarYMax * scrollBoxYMax,
-scrollBoxYMax, 0);
scrollBarY = constants.scrollBarMargin -
scrollBoxY / scrollBoxYMax * scrollBarYMax;
scrollHandler(scrollBarY, scrollBoxY);
d3.event.preventDefault();
});

scrollBar.on('.drag',null);
scrollBox.on('.drag',null);
var drag = d3.behavior.drag()
.on('drag', function() {
scrollHandler(d3.event.dy, scrollheight);
});
var drag = d3.behavior.drag().on('drag', function() {
scrollBarY = Lib.constrain(
d3.event.y - constants.scrollBarHeight / 2,
constants.scrollBarMargin,
constants.scrollBarMargin + scrollBarYMax);
scrollBoxY = - (scrollBarY - constants.scrollBarMargin) /
scrollBarYMax * scrollBoxYMax;
scrollHandler(scrollBarY, scrollBoxY);
});

scrollBar.call(drag);
scrollBox.call(drag);

}


function scrollHandler(delta, scrollheight) {

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;

function scrollHandler(scrollBarY, scrollBoxY) {
scrollBox.attr('data-scroll', scrollBoxY);
scrollBox.attr('transform', 'translate(0, ' + scrollBoxY + ')');
scrollBar.call(
Drawing.setRect,
opts.width - (constants.scrollBarWidth + constants.scrollBarMargin),
opts.width,
scrollBarY,
constants.scrollBarWidth,
constants.scrollBarHeight
Expand Down Expand Up @@ -347,7 +382,10 @@ function drawTexts(context, gd, d, i, traces) {

function textLayout(s) {
Plotly.util.convertToTspans(s, function() {
if(gd.firstRender) repositionLegend(gd, traces);
if(gd.firstRender) {
computeLegendDimensions(gd, traces);
expandMargin(gd);
}
});
s.selectAll('tspan.line').attr({x: s.attr('x')});
}
Expand All @@ -366,9 +404,8 @@ function drawTexts(context, gd, d, i, traces) {
else text.call(textLayout);
}

function repositionLegend(gd, traces) {
function computeLegendDimensions(gd, traces) {
var fullLayout = gd._fullLayout,
gs = fullLayout._size,
opts = fullLayout.legend,
borderwidth = opts.borderwidth;

Expand Down Expand Up @@ -420,7 +457,6 @@ function repositionLegend(gd, traces) {
opts.width = Math.max(opts.width, tWidth || 0);
});


opts.width += 45 + borderwidth * 2;
opts.height += 10 + borderwidth * 2;

Expand All @@ -431,41 +467,31 @@ function repositionLegend(gd, traces) {
traces.selectAll('.legendtoggle')
.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
// make sure we're only getting full pixels
opts.width = Math.ceil(opts.width);
opts.height = Math.ceil(opts.height);
}

var lx = gs.l + gs.w * opts.x,
ly = gs.t + gs.h * (1-opts.y);
function expandMargin(gd) {
var fullLayout = gd._fullLayout,
opts = fullLayout.legend;

var xanchor = 'left';
if(anchorUtils.isRightAnchor(opts)) {
lx -= opts.width;
xanchor = 'right';
}
if(anchorUtils.isCenterAnchor(opts)) {
lx -= opts.width / 2;
else if(anchorUtils.isCenterAnchor(opts)) {
xanchor = 'center';
}

var yanchor = 'top';
if(anchorUtils.isBottomAnchor(opts)) {
ly -= opts.height;
yanchor = 'bottom';
}
if(anchorUtils.isMiddleAnchor(opts)) {
ly -= opts.height / 2;
else if(anchorUtils.isMiddleAnchor(opts)) {
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', {
x: opts.x,
Expand All @@ -476,3 +502,26 @@ function repositionLegend(gd, traces) {
t: opts.height * ({bottom: 1, middle: 0.5}[yanchor] || 0)
});
}

function expandHorizontalMargin(gd) {
var fullLayout = gd._fullLayout,
opts = fullLayout.legend;

var xanchor = 'left';
if(anchorUtils.isRightAnchor(opts)) {
xanchor = 'right';
}
else if(anchorUtils.isCenterAnchor(opts)) {
xanchor = 'center';
}

// lastly check if the margin auto-expand has changed
Plots.autoMargin(gd, 'legend', {
x: opts.x,
y: 0.5,
l: opts.width * ({right: 1, center: 0.5}[xanchor] || 0),
r: opts.width * ({left: 1, center: 0.5}[xanchor] || 0),
b: 0,
t: 0
});
}
Binary file added test/image/baselines/legend_negative_y.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
62 changes: 62 additions & 0 deletions test/image/mocks/legend_negative_y.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
{
"data":[
{
"x":[
0,
1,
2,
3,
4,
5,
6,
7,
8
],
"y":[
0,
3,
6,
4,
5,
2,
3,
5,
4
],
"type":"scatter"
},
{
"x":[
0,
1,
2,
3,
4,
5,
6,
7,
8
],
"y":[
0,
4,
7,
8,
3,
6,
3,
3,
4
],
"type":"scatter"
}
],
"layout":{
"showlegend":true,
"legend":{
"x":1,
"y":-1,
"xanchor":"left"
}
}
}
26 changes: 18 additions & 8 deletions test/jasmine/tests/legend_scroll_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var Plotly = require('@lib/index');
var Lib = require('@src/lib');
var constants = require('@src/components/legend/constants');

var createGraph = require('../assets/create_graph_div');
var destroyGraph = require('../assets/destroy_graph_div');
Expand Down Expand Up @@ -55,14 +56,23 @@ describe('The legend', function() {
});

it('should scroll when there\'s a wheel event', function() {
var scrollBox = legend.getElementsByClassName('scrollbox')[0];

legend.dispatchEvent(scrollTo(100));

// Compare against -5 because of a scroll factor of 20
// ( 100 / 20 === 5 )
expect(scrollBox.getAttribute('transform')).toBe('translate(0, -5)');
expect(scrollBox.getAttribute('data-scroll')).toBe('-5');
var scrollBox = legend.getElementsByClassName('scrollbox')[0],
legendHeight = getBBox(legend).height,
scrollBoxYMax = gd._fullLayout.legend.height - legendHeight,
scrollBarYMax = legendHeight -
constants.scrollBarHeight -
2 * constants.scrollBarMargin,
initialDataScroll = scrollBox.getAttribute('data-scroll'),
wheelDeltaY = 100,
finalDataScroll = '' + Lib.constrain(initialDataScroll -
wheelDeltaY / scrollBarYMax * scrollBoxYMax,
-scrollBoxYMax, 0);

legend.dispatchEvent(scrollTo(wheelDeltaY));

expect(scrollBox.getAttribute('data-scroll')).toBe(finalDataScroll);
expect(scrollBox.getAttribute('transform')).toBe(
'translate(0, ' + finalDataScroll + ')');
});

it('should constrain scrolling to the contents', function() {
Expand Down