Skip to content

Fix overlap of legend border and scroll box #478

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 19 additions & 14 deletions src/components/legend/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,28 +223,29 @@ module.exports = function draw(gd) {
legendHeight = Math.min(lyMax - ly, opts.height);
}

// Deal with scrolling
// Set size and position of all the elements that make up a legend:
// legend, background and border, scroll box and scroll bar
legend.attr('transform', 'translate(' + lx + ',' + ly + ')');

bg.attr({
width: opts.width - opts.borderwidth,
height: legendHeight - opts.borderwidth,
x: opts.borderwidth / 2,
y: opts.borderwidth / 2
});

var scrollPosition = scrollBox.attr('data-scroll') || 0;

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

bg.attr({
clipPath.select('rect').attr({
width: opts.width - 2 * opts.borderwidth,
height: legendHeight - 2 * opts.borderwidth,
x: opts.borderwidth,
x: opts.borderwidth - scrollPosition,
y: opts.borderwidth
});

legend.attr('transform', 'translate(' + lx + ',' + ly + ')');

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

legend.call(Drawing.setClipUrl, clipId);
scrollBox.call(Drawing.setClipUrl, clipId);

// If scrollbar should be shown.
if(opts.height - legendHeight > 0 && !gd._context.staticPlot) {
Expand All @@ -259,7 +260,8 @@ module.exports = function draw(gd) {
});

clipPath.select('rect').attr({
width: opts.width +
width: opts.width -
2 * opts.borderwidth +
constants.scrollBarWidth +
constants.scrollBarMargin
});
Expand Down Expand Up @@ -318,6 +320,9 @@ module.exports = function draw(gd) {
constants.scrollBarWidth,
constants.scrollBarHeight
);
clipPath.select('rect').attr({
y: opts.borderwidth - scrollBoxY
});
}

if(gd._context.editable) {
Expand Down
Binary file modified test/image/baselines/17.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/20.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/legend_scroll.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/legend_style.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/text_export.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
27 changes: 16 additions & 11 deletions test/jasmine/tests/legend_scroll_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var mock = require('../../image/mocks/legend_scroll.json');
describe('The legend', function() {
'use strict';

var gd, legend;
var gd, legend, bg;

function countLegendGroups(gd) {
return gd._fullLayout._toppaper.selectAll('g.legend').size();
Expand All @@ -27,6 +27,9 @@ describe('The legend', function() {
return gd._fullLayout.height - gd._fullLayout.margin.t - gd._fullLayout.margin.b;
}

function getLegendHeight() {
return gd._fullLayout.legend.borderwidth + getBBox(bg).height;
}

describe('when plotted with many traces', function() {
beforeEach(function(done) {
Expand All @@ -36,14 +39,15 @@ describe('The legend', function() {

Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(function() {
legend = document.getElementsByClassName('legend')[0];
bg = document.getElementsByClassName('bg')[0];
done();
});
});

afterEach(destroyGraph);

it('should not exceed plot height', function() {
var legendHeight = getBBox(legend).height;
var legendHeight = getLegendHeight();

expect(+legendHeight).toBe(getPlotHeight(gd));
});
Expand All @@ -57,7 +61,7 @@ describe('The legend', function() {

it('should scroll when there\'s a wheel event', function() {
var scrollBox = legend.getElementsByClassName('scrollbox')[0],
legendHeight = getBBox(legend).height,
legendHeight = getLegendHeight(),
scrollBoxYMax = gd._fullLayout.legend.height - legendHeight,
scrollBarYMax = legendHeight -
constants.scrollBarHeight -
Expand Down Expand Up @@ -87,7 +91,7 @@ describe('The legend', function() {

it('should scale the scrollbar movement from top to bottom', function() {
var scrollBar = legend.getElementsByClassName('scrollbar')[0],
legendHeight = getBBox(legend).height;
legendHeight = getLegendHeight();

// The scrollbar is 20px tall and has 4px margins

Expand All @@ -111,10 +115,10 @@ describe('The legend', function() {
});

it('should resize when relayout\'ed with new height', function(done) {
var origLegendHeight = getBBox(legend).height;
var origLegendHeight = getLegendHeight();

Plotly.relayout(gd, 'height', gd._fullLayout.height / 2).then(function() {
var legendHeight = getBBox(legend).height;
var legendHeight = getLegendHeight();

//legend still exists and not duplicated
expect(countLegendGroups(gd)).toBe(1);
Expand Down Expand Up @@ -164,14 +168,15 @@ describe('The legend', function() {
});

it('should resize when traces added', function(done) {
var origLegend = document.getElementsByClassName('legend')[0];
var origLegendHeight = getBBox(origLegend).height;
legend = document.getElementsByClassName('legend')[0];
bg = document.getElementsByClassName('bg')[0];
var origLegendHeight = getLegendHeight();

Plotly.addTraces(gd, { x: [1,2,3], y: [4,3,2], name: 'Test2' }).then(function() {
var legend = document.getElementsByClassName('legend')[0];
var legendHeight = getBBox(legend).height;
legend = document.getElementsByClassName('legend')[0];
bg = document.getElementsByClassName('bg')[0];
var legendHeight = getLegendHeight();

// clippath resized to show new trace
expect(+legendHeight).toBeCloseTo(+origLegendHeight + 19, 0);

done();
Expand Down