-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Bar text constraint configuration #1931
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 7 commits
969cac6
013c871
b20ce67
3e92d7a
06bd80d
5d67212
f690d9b
30bba26
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 |
---|---|---|
|
@@ -237,20 +237,22 @@ function appendBarText(gd, bar, calcTrace, i, x0, x1, y0, y1) { | |
} | ||
|
||
// compute text transform | ||
var transform; | ||
var transform, constrained; | ||
if(textPosition === 'outside') { | ||
constrained = trace.constraintext === 'both' || trace.constraintext === 'outside'; | ||
transform = getTransformToMoveOutsideBar(x0, x1, y0, y1, textBB, | ||
orientation); | ||
orientation, constrained); | ||
} | ||
else { | ||
constrained = trace.constraintext === 'both' || trace.constraintext === 'inside'; | ||
transform = getTransformToMoveInsideBar(x0, x1, y0, y1, textBB, | ||
orientation); | ||
orientation, constrained); | ||
} | ||
|
||
textSelection.attr('transform', transform); | ||
} | ||
|
||
function getTransformToMoveInsideBar(x0, x1, y0, y1, textBB, orientation) { | ||
function getTransformToMoveInsideBar(x0, x1, y0, y1, textBB, orientation, constrained) { | ||
// compute text and target positions | ||
var textWidth = textBB.width, | ||
textHeight = textBB.height, | ||
|
@@ -289,12 +291,12 @@ function getTransformToMoveInsideBar(x0, x1, y0, y1, textBB, orientation) { | |
else if((textWidth < textHeight) === (barWidth < barHeight)) { | ||
// only scale is required | ||
rotate = false; | ||
scale = Math.min(barWidth / textWidth, barHeight / textHeight); | ||
scale = constrained ? Math.min(barWidth / textWidth, barHeight / textHeight) : 1; | ||
} | ||
else { | ||
// both scale and rotation are required | ||
rotate = true; | ||
scale = Math.min(barHeight / textWidth, barWidth / textHeight); | ||
scale = constrained ? Math.min(barHeight / textWidth, barWidth / textHeight) : 1; | ||
} | ||
|
||
if(rotate) rotate = 90; // rotate clockwise | ||
|
@@ -335,23 +337,26 @@ function getTransformToMoveInsideBar(x0, x1, y0, y1, textBB, orientation) { | |
return getTransform(textX, textY, targetX, targetY, scale, rotate); | ||
} | ||
|
||
function getTransformToMoveOutsideBar(x0, x1, y0, y1, textBB, orientation) { | ||
function getTransformToMoveOutsideBar(x0, x1, y0, y1, textBB, orientation, constrained) { | ||
var barWidth = (orientation === 'h') ? | ||
Math.abs(y1 - y0) : | ||
Math.abs(x1 - x0), | ||
textpad; | ||
|
||
// apply text padding if possible | ||
// Keep the padding so the text doesn't sit right against | ||
// the bars, but don't factor it into barWidth | ||
if(barWidth > 2 * TEXTPAD) { | ||
textpad = TEXTPAD; | ||
barWidth -= 2 * textpad; | ||
} | ||
|
||
// compute rotation and scale | ||
var rotate = false, | ||
var rotate = false; | ||
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 may be a silly question but... why do we have an 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. Good catch. I'll clean it up. 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. 🔪 'd |
||
var scale = 1; | ||
if(constrained) { | ||
scale = (orientation === 'h') ? | ||
Math.min(1, barWidth / textBB.height) : | ||
Math.min(1, barWidth / textBB.width); | ||
} | ||
|
||
// compute text and target positions | ||
var textX = (textBB.left + textBB.right) / 2, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,7 @@ describe('Bar.supplyDefaults', function() { | |
expect(traceOut.texfont).toBeUndefined(); | ||
expect(traceOut.insidetexfont).toBeUndefined(); | ||
expect(traceOut.outsidetexfont).toBeUndefined(); | ||
expect(traceOut.constraintext).toBe(); | ||
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 looks odd... 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. Oops. Good catch. 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. ✅ |
||
}); | ||
|
||
it('should default textfont to layout.font', function() { | ||
|
@@ -116,6 +117,7 @@ describe('Bar.supplyDefaults', function() { | |
expect(traceOut.insidetextfont).not.toBe(layout.font); | ||
expect(traceOut.insidetextfont).not.toBe(traceOut.textfont); | ||
expect(traceOut.outsidetexfont).toBeUndefined(); | ||
expect(traceOut.constraintext).toBe('both'); | ||
}); | ||
|
||
it('should inherit layout.calendar', function() { | ||
|
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.
Ah, good call.