-
-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(bar): prevent outside text labels from overlapping tilted axis ticks #7872
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
base: master
Are you sure you want to change the base?
Changes from all commits
dbc9a51
8c78dd6
9ea39bc
6b1363e
73c8ab5
a61530b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Prevent outside bar text labels from overlapping tilted axis ticks [[#7872](https://github.com/plotly/plotly.js/pull/7872)] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -554,6 +554,7 @@ function appendBarText(gd, plotinfo, bar, cd, i, x0, x1, y0, y1, r, overhead, op | |||||||||||||||||||||||||||||||||||||||
| // get trace attributes | ||||||||||||||||||||||||||||||||||||||||
| var trace = cd[0].trace; | ||||||||||||||||||||||||||||||||||||||||
| var isHorizontal = trace.orientation === 'h'; | ||||||||||||||||||||||||||||||||||||||||
| var zeroBarDir = getZeroBarDir(cd, isHorizontal, xa, ya); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| var text = getText(fullLayout, cd, i, xa, ya); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -707,7 +708,10 @@ function appendBarText(gd, plotinfo, bar, cd, i, x0, x1, y0, y1, r, overhead, op | |||||||||||||||||||||||||||||||||||||||
| transform = toMoveOutsideBar(x0, x1, y0, y1, textBB, { | ||||||||||||||||||||||||||||||||||||||||
| isHorizontal: isHorizontal, | ||||||||||||||||||||||||||||||||||||||||
| constrained: constrained, | ||||||||||||||||||||||||||||||||||||||||
| angle: angle | ||||||||||||||||||||||||||||||||||||||||
| angle: angle, | ||||||||||||||||||||||||||||||||||||||||
| xa: xa, // Pass the X-Axis configuration | ||||||||||||||||||||||||||||||||||||||||
| ya: ya, // Pass the Y-Axis configuration | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+711
to
+713
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these are also no longer needed
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| zeroBarDir: zeroBarDir | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||
| constrained = trace.constraintext === 'both' || trace.constraintext === 'inside'; | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -957,12 +961,35 @@ function toMoveOutsideBar(x0, x1, y0, y1, textBB, opts) { | |||||||||||||||||||||||||||||||||||||||
| var anchorX = 0; | ||||||||||||||||||||||||||||||||||||||||
| var anchorY = 0; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Dynamic presentation safety buffer to clear tilted axis tick labels | ||||||||||||||||||||||||||||||||||||||||
| var axisPad = 0; | ||||||||||||||||||||||||||||||||||||||||
| if (!isHorizontal && opts.xa && opts.xa.side === 'top') { | ||||||||||||||||||||||||||||||||||||||||
| if (opts.xa._g && opts.xa._g.node()) { | ||||||||||||||||||||||||||||||||||||||||
| var axisBB = opts.xa._g.node().getBBox(); | ||||||||||||||||||||||||||||||||||||||||
| if (axisBB && axisBB.height > 0) { | ||||||||||||||||||||||||||||||||||||||||
| // Shift exactly past the bounding height of the tilted labels plus a clean 6px visual gap | ||||||||||||||||||||||||||||||||||||||||
| axisPad = axisBB.height + 6; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } else if (isHorizontal && opts.ya && opts.ya.side === 'right') { | ||||||||||||||||||||||||||||||||||||||||
| if (opts.ya._g && opts.ya._g.node()) { | ||||||||||||||||||||||||||||||||||||||||
| var axisBB = opts.ya._g.node().getBBox(); | ||||||||||||||||||||||||||||||||||||||||
| if (axisBB && axisBB.width > 0) { | ||||||||||||||||||||||||||||||||||||||||
| // Shift exactly past the bounding width of the side labels plus a clean 6px visual gap | ||||||||||||||||||||||||||||||||||||||||
| axisPad = axisBB.width + 6; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+964
to
+983
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this can all be removed now that the Along with removing the references to
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| var dir = isHorizontal ? dirSign(x1, x0) : dirSign(y0, y1); | ||||||||||||||||||||||||||||||||||||||||
| if ((isHorizontal ? x0 === x1 : y0 === y1) && opts.zeroBarDir) { | ||||||||||||||||||||||||||||||||||||||||
| dir = opts.zeroBarDir; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
984
to
+987
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is logically equivalent, just a style suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| if (isHorizontal) { | ||||||||||||||||||||||||||||||||||||||||
| targetX = x1 - dir * textpad; | ||||||||||||||||||||||||||||||||||||||||
| targetX = x1 - dir * (textpad + axisPad); | ||||||||||||||||||||||||||||||||||||||||
| anchorX = dir * extrapad; | ||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||
| targetY = y1 + dir * textpad; | ||||||||||||||||||||||||||||||||||||||||
| targetY = y1 + dir * (textpad + axisPad); | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+989
to
+992
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| anchorY = -dir * extrapad; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -999,6 +1026,38 @@ function getTextPosition(trace, index) { | |||||||||||||||||||||||||||||||||||||||
| return helpers.coerceEnumerated(attributeTextPosition, value); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| function getZeroBarDir(cd, isHorizontal, xa, ya) { | ||||||||||||||||||||||||||||||||||||||||
| var hasPositive = false; | ||||||||||||||||||||||||||||||||||||||||
| var hasNegative = false; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| for (var i = 0; i < cd.length; i++) { | ||||||||||||||||||||||||||||||||||||||||
| var s = cd[i].s; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (s > 0) { | ||||||||||||||||||||||||||||||||||||||||
| hasPositive = true; | ||||||||||||||||||||||||||||||||||||||||
| } else if (s < 0) { | ||||||||||||||||||||||||||||||||||||||||
| hasNegative = true; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (hasPositive && hasNegative) { | ||||||||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| var axis = isHorizontal ? xa : ya; | ||||||||||||||||||||||||||||||||||||||||
| var positiveDir = -dirSign(axis.range[0], axis.range[1]); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (!hasNegative) { | ||||||||||||||||||||||||||||||||||||||||
| return positiveDir; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (!hasPositive) { | ||||||||||||||||||||||||||||||||||||||||
| return -positiveDir; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| function calcTexttemplate(fullLayout, cd, index, xa, ya) { | ||||||||||||||||||||||||||||||||||||||||
| var trace = cd[0].trace; | ||||||||||||||||||||||||||||||||||||||||
| var texttemplate = Lib.castOption(trace, index, 'texttemplate'); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1160,5 +1219,6 @@ function calcTextinfo(cd, index, xa, ya) { | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| module.exports = { | ||||||||||||||||||||||||||||||||||||||||
| plot: plot, | ||||||||||||||||||||||||||||||||||||||||
| toMoveInsideBar: toMoveInsideBar | ||||||||||||||||||||||||||||||||||||||||
| toMoveInsideBar: toMoveInsideBar, | ||||||||||||||||||||||||||||||||||||||||
| toMoveOutsideBar: toMoveOutsideBar | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this can also be removed
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1411,6 +1411,34 @@ describe('A bar plot', function() { | |
| .then(done, done.fail); | ||
| }); | ||
|
|
||
| it('should keep zero-value outside labels on the same side as negative bars', function(done) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice test! |
||
| var data = [{ | ||
| y: [-10, 0, -30], | ||
| type: 'bar', | ||
| text: ['a', 'zero', 'c'], | ||
| textposition: 'outside' | ||
| }]; | ||
|
|
||
| Plotly.newPlot(gd, data).then(function() { | ||
| var traceNodes = getAllTraceNodes(gd); | ||
| var barNodes = getAllBarNodes(traceNodes[0]); | ||
| var foundTextNodes; | ||
|
|
||
| for(var i = 0; i < barNodes.length; i++) { | ||
| var barNode = barNodes[i]; | ||
| var pathNode = barNode.querySelector('path'); | ||
| var textNode = barNode.querySelector('text'); | ||
| if(textNode) { | ||
| foundTextNodes = true; | ||
| assertTextIsBelowPath(textNode, pathNode); | ||
| } | ||
| } | ||
|
|
||
| expect(foundTextNodes).toBe(true); | ||
| }) | ||
| .then(done, done.fail); | ||
| }); | ||
|
|
||
| it('should show bar texts (horizontal case)', function(done) { | ||
| var data = [{ | ||
| x: [10, -20, 30], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.