From 869e054820323f89335e451627d38d6bc8103fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 5 Mar 2018 09:17:04 +0100 Subject: [PATCH 1/2] Fix layout of centered hover info #2154 - Bug: when a tooltip displays a sufficiently long text (also depends on overall plot dimensions), the tooltip isn't rendered right or left but centered. In this case the tooltip text was partly rendered outside the tooltip box and the secondary label (the trace name) was hidden underneath the primary label. --- src/components/fx/hover.js | 5 +- test/jasmine/tests/hover_label_test.js | 90 ++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 3d326d007cc..cd78c93aebc 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -1126,7 +1126,7 @@ function alignHoverText(hoverLabels, rotateLabels) { offsetY = d.offset; if(d.anchor === 'middle') { txx -= d.tx2width / 2; - tx2x -= d.tx2width / 2; + tx2x += d.txwidth / 2 + HOVERTEXTPAD; } if(rotateLabels) { offsetY *= -YSHIFTY; @@ -1135,7 +1135,8 @@ function alignHoverText(hoverLabels, rotateLabels) { g.select('path').attr('d', d.anchor === 'middle' ? // middle aligned: rect centered on data - ('M-' + (d.bx / 2) + ',-' + (d.by / 2) + 'h' + d.bx + 'v' + d.by + 'h-' + d.bx + 'Z') : + ('M-' + (d.bx / 2 + d.tx2width / 2) + ',-' + (d.by / 2) + + 'h' + d.bx + 'v' + d.by + 'h-' + d.bx + 'Z') : // left or right aligned: side rect with arrow to data ('M0,0L' + (horzSign * HOVERARROWSIZE + offsetX) + ',' + (HOVERARROWSIZE + offsetY) + 'v' + (d.by / 2 - HOVERARROWSIZE) + diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index d0b16cb905c..f1c4eb370e7 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1109,6 +1109,96 @@ describe('hover info', function() { }); }); + + describe('centered', function() { + var trace1 = { + x: ['giraffes'], + y: [5], + name: 'LA Zoo', + type: 'bar', + text: ['Way too long hover info!'] + }; + var trace2 = { + x: ['giraffes'], + y: [5], + name: 'SF Zoo', + type: 'bar', + text: ['San Francisco'] + }; + var data = [trace1, trace2]; + var layout = {width: 600, height: 300, barmode: 'stack'}; + + var gd; + + beforeEach(function(done) { + gd = createGraphDiv(); + Plotly.plot(gd, data, layout).then(done); + }); + + function centeredHoverInfoNodes() { + var g = d3.selectAll('g.hoverlayer g.hovertext').filter(function() { + return !d3.select(this).select('[data-unformatted="LA Zoo"]').empty(); + }); + + return { + primaryText: g.select('text:not([data-unformatted="LA Zoo"])').node(), + primaryBox: g.select('path').node(), + secondaryText: g.select('[data-unformatted="LA Zoo"]').node(), + secondaryBox: g.select('rect').node() + }; + } + + function ensureCentered(hoverInfoNodes) { + expect(hoverInfoNodes.primaryText.getAttribute('text-anchor')).toBe('middle'); + expect(hoverInfoNodes.secondaryText.getAttribute('text-anchor')).toBe('middle'); + return hoverInfoNodes; + } + + function assertElemInside(elem, container, msg) { + var elemBB = elem.getBoundingClientRect(); + var contBB = container.getBoundingClientRect(); + expect(contBB.left < elemBB.left && + contBB.right > elemBB.right && + contBB.top < elemBB.top && + contBB.bottom > elemBB.bottom).toBe(true, msg); + } + + function assertElemRightTo(elem, refElem, msg) { + var elemBB = elem.getBoundingClientRect(); + var refElemBB = refElem.getBoundingClientRect(); + expect(elemBB.left >= refElemBB.right).toBe(true, msg); + } + + function assertTopsAligned(elem1, elem2, msg) { + var elem1BB = elem1.getBoundingClientRect(); + var elem2BB = elem2.getBoundingClientRect(); + + // Hint: toBeWithin tolerance is exclusive, hence a + // diff of exactly 1 would fail the test + var tolerance = 1.1; + expect(elem1BB.top - elem2BB.top).toBeWithin(0, tolerance, msg); + } + + it('renders labels inside boxes', function(done) { + _hover(gd, 300, 150); + + var nodes = ensureCentered(centeredHoverInfoNodes()); + assertElemInside(nodes.primaryText, nodes.primaryBox, 'Primary text inside box'); + assertElemInside(nodes.secondaryText, nodes.secondaryBox, 'Secondary text inside box'); + + done(); + }); + + it('renders secondary info box right to primary info box', function(done) { + _hover(gd, 300, 150); + + var nodes = ensureCentered(centeredHoverInfoNodes()); + assertElemRightTo(nodes.secondaryBox, nodes.primaryBox, 'Secondary box right to primary box'); + assertTopsAligned(nodes.secondaryBox, nodes.primaryBox, 'Top edges of primary and secondary boxes aligned'); + + done(); + }); + }); }); describe('hover info on stacked subplots', function() { From 0dd85fa0bb4bc1966b56a58aca35a8ca2b00841e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 5 Mar 2018 16:24:16 +0100 Subject: [PATCH 2/2] Let centered hover info tests be synchronous #2154 - No reason to run them asynchronously. --- test/jasmine/tests/hover_label_test.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index f1c4eb370e7..d279891317f 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1179,24 +1179,20 @@ describe('hover info', function() { expect(elem1BB.top - elem2BB.top).toBeWithin(0, tolerance, msg); } - it('renders labels inside boxes', function(done) { + it('renders labels inside boxes', function() { _hover(gd, 300, 150); var nodes = ensureCentered(centeredHoverInfoNodes()); assertElemInside(nodes.primaryText, nodes.primaryBox, 'Primary text inside box'); assertElemInside(nodes.secondaryText, nodes.secondaryBox, 'Secondary text inside box'); - - done(); }); - it('renders secondary info box right to primary info box', function(done) { + it('renders secondary info box right to primary info box', function() { _hover(gd, 300, 150); var nodes = ensureCentered(centeredHoverInfoNodes()); assertElemRightTo(nodes.secondaryBox, nodes.primaryBox, 'Secondary box right to primary box'); assertTopsAligned(nodes.secondaryBox, nodes.primaryBox, 'Top edges of primary and secondary boxes aligned'); - - done(); }); }); });