-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Contour label extra pad and correct minus sign #4540
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
Changes from 2 commits
a32f76f
30bee29
7cbb5ce
6429830
f5f4d62
bf97aa1
b5c85b1
fd0b9fb
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 |
---|---|---|
|
@@ -26,6 +26,7 @@ var convertToConstraints = require('./convert_to_constraints'); | |
var closeBoundaries = require('./close_boundaries'); | ||
var constants = require('./constants'); | ||
var costConstants = constants.LABELOPTIMIZER; | ||
var LABEL_LINE_CLIP_PAD = constants.LABEL_LINE_CLIP_PAD; | ||
|
||
exports.plot = function plot(gd, plotinfo, cdcontours, contourLayer) { | ||
var xa = plotinfo.xaxis; | ||
|
@@ -394,21 +395,21 @@ exports.labelFormatter = function(gd, cd0) { | |
var trace = cd0.trace; | ||
var contours = trace.contours; | ||
|
||
var formatAxis = { | ||
type: 'linear', | ||
_id: 'ycontour', | ||
showexponent: 'all', | ||
exponentformat: 'B' | ||
}; | ||
|
||
if(contours.labelformat) { | ||
return fullLayout._d3locale.numberFormat(contours.labelformat); | ||
formatAxis.tickformat = contours.labelformat; | ||
setConvert(formatAxis, fullLayout); | ||
} else { | ||
var formatAxis; | ||
var cOpts = Colorscale.extractOpts(trace); | ||
if(cOpts && cOpts.colorbar && cOpts.colorbar._axis) { | ||
formatAxis = cOpts.colorbar._axis; | ||
} else { | ||
formatAxis = { | ||
type: 'linear', | ||
_id: 'ycontour', | ||
showexponent: 'all', | ||
exponentformat: 'B' | ||
}; | ||
|
||
if(contours.type === 'constraint') { | ||
var value = contours.value; | ||
if(Array.isArray(value)) { | ||
|
@@ -429,10 +430,9 @@ exports.labelFormatter = function(gd, cd0) { | |
formatAxis._tmin = null; | ||
formatAxis._tmax = null; | ||
} | ||
return function(v) { | ||
return Axes.tickText(formatAxis, v).text; | ||
}; | ||
} | ||
|
||
return function(v) { return Axes.tickText(formatAxis, v).text; }; | ||
}; | ||
|
||
exports.calcTextOpts = function(level, contourFormat, dummyText, gd) { | ||
|
@@ -544,8 +544,8 @@ function locationCost(loc, textOpts, labelData, bounds) { | |
} | ||
|
||
exports.addLabelData = function(loc, textOpts, labelData, labelClipPathData) { | ||
var halfWidth = textOpts.width / 2; | ||
var halfHeight = textOpts.height / 2; | ||
var halfWidth = textOpts.width / 2 + LABEL_LINE_CLIP_PAD; | ||
var halfHeight = textOpts.height / 2 + LABEL_LINE_CLIP_PAD; | ||
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. After viewing the baseline changes, I think we may not need to add vertical padding to the height. 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 agree, let's not add vertical padding. In fact if there was a way to make it even a tighter fit vertically than it currently is on master, I'd be all for it. Probably not though, assuming that comes from a bounding box... but the goal is to avoid clipping neighboring contour lines as much as possible, since that can cause confusion about which line is actually being labeled. |
||
|
||
var x = loc.x; | ||
var y = loc.y; | ||
|
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.
Could you please try 3px horizontal padding here?
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.
2 px looks like plenty to me, but I might argue for making it a fraction of the font size (or text height) so that it scales with the text.