Skip to content

Workarounds for "common" (aka axis) hover label clipping #4298

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 3 commits into from
Oct 25, 2019
Merged

Workarounds for "common" (aka axis) hover label clipping #4298

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68521df
constraint common hovermode:'x' hover labels into graph viewport
etpinard Oct 22, 2019
2c94e5f
mv and clip text in common hovermode:'y' hover label
etpinard Oct 22, 2019
8d79522
shift multi-line <tspan> in common hovermode:'y' hover label
etpinard Oct 24, 2019
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
37 changes: 27 additions & 10 deletions src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,18 +772,19 @@ function createHoverText(hoverData, opts, gd) {
var commonBgColor = commonLabelOpts.bgcolor || Color.defaultLine;
var commonStroke = commonLabelOpts.bordercolor || Color.contrast(commonBgColor);
var contrastColor = Color.contrast(commonBgColor);
var commonLabelFont = {
family: commonLabelOpts.font.family || fontFamily,
size: commonLabelOpts.font.size || fontSize,
color: commonLabelOpts.font.color || contrastColor
};

lpath.style({
fill: commonBgColor,
stroke: commonStroke
});

ltext.text(t0)
.call(Drawing.font,
commonLabelOpts.font.family || fontFamily,
commonLabelOpts.font.size || fontSize,
commonLabelOpts.font.color || contrastColor
)
.call(Drawing.font, commonLabelFont)
.call(svgTextUtils.positionText, 0, 0)
.call(svgTextUtils.convertToTspans, gd);

Expand Down Expand Up @@ -861,23 +862,39 @@ function createHoverText(hoverData, opts, gd) {
'H' + leftsign + HOVERARROWSIZE + 'V-' + HOVERARROWSIZE + 'Z');

var halfHeight = tbb.height / 2;
var lty = outerTop - tbb.top - halfHeight;
var clipId = 'clip' + fullLayout._uid + 'commonlabel' + ya._id;
var clipPath;
var ltx;

if(lx < (tbb.width + 2 * HOVERTEXTPAD + HOVERARROWSIZE)) {
Copy link
Contributor Author

@etpinard etpinard Oct 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolaskruchten this used to be:

if(lx < (tbb.width + HOVERTEXTPAD + HOVERARROWSIZE)) { // ...

which may have caused the problem described in #4221 (comment)

New attempt: https://codepen.io/etpinard/pen/qBBrLwx

ltx = tbb.width - lx + HOVERTEXTPAD;
clipPath = 'M-' + (HOVERARROWSIZE + HOVERTEXTPAD) + '-' + halfHeight +
'h-' + (tbb.width - HOVERTEXTPAD) +
'V' + halfHeight +
'h' + (tbb.width - HOVERTEXTPAD) + 'Z';

var ltx = tbb.width - lx + HOVERTEXTPAD;
svgTextUtils.positionText(ltext, ltx, lty);

// shift each line (except the longest) so that start-of-line
// is always visible
if(anchor === 'end') {
ltext.selectAll('tspan').each(function() {
var s = d3.select(this);
var dummy = Drawing.tester.append('text')
.text(s.text())
.call(Drawing.font, commonLabelFont);
var dummyBB = dummy.node().getBoundingClientRect();
if(dummyBB.width < tbb.width) {
s.attr('x', ltx - dummyBB.width);
}
dummy.remove();
});
}
} else {
ltx = sgn * (HOVERTEXTPAD + HOVERARROWSIZE);
svgTextUtils.positionText(ltext, sgn * (HOVERTEXTPAD + HOVERARROWSIZE), lty);
clipPath = null;
}

svgTextUtils.positionText(ltext, ltx, outerTop - tbb.top - halfHeight);

var textClip = fullLayout._topclips.selectAll('#' + clipId).data(clipPath ? [0] : []);
textClip.enter().append('clipPath').attr('id', clipId).append('path');
textClip.exit().remove();
Expand Down
24 changes: 24 additions & 0 deletions test/jasmine/tests/hover_label_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,6 +1767,18 @@ describe('hover info', function() {
var clipId = 'clip' + fullLayout._uid + 'commonlabely';
var clipPath = d3.select('#' + clipId);
negateIf(exp.clip, expect(clipPath.node())).toBe(null, 'text clip path|' + msg);

if(exp.tspanX) {
var tspans = label.selectAll('tspan');
if(tspans.size()) {
tspans.each(function(d, i) {
var s = d3.select(this);
expect(s.attr('x')).toBeWithin(exp.tspanX[i], 5, i + '- tspan shift| ' + msg);
});
} else {
fail('fail to generate tspans in hover label');
}
}
} else {
fail('fail to generate common hover label');
}
Expand All @@ -1790,6 +1802,18 @@ describe('hover info', function() {
.then(_assert('on way long label', {txt: 'Waay loooong label', clip: true, ltx: 38}))
.then(_hoverA)
.then(_assert('on "a" label', {txt: 'a', clip: false, ltx: -9}))
.then(function() {
return Plotly.restyle(gd, {
y: [['Looong label', 'Loooooger label', 'SHORT!<br>Waay loooong label', 'a']]
});
})
.then(_hoverWayLong)
.then(_assert('on way long label (multi-line case)', {
txt: 'SHORT!Waay loooong label',
clip: true,
ltx: 38,
tspanX: [-11, 38]
}))
.catch(failTest)
.then(done);
});
Expand Down