-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Parcoords fonts #1624
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
Parcoords fonts #1624
Changes from all commits
505614d
9939241
a8e444a
3925d6d
a3544c7
c0e0845
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 |
---|---|---|
|
@@ -12,6 +12,7 @@ var lineLayerMaker = require('./lines'); | |
var c = require('./constants'); | ||
var Lib = require('../../lib'); | ||
var d3 = require('d3'); | ||
var Drawing = require('../../components/drawing'); | ||
|
||
|
||
function keyFun(d) {return d.key;} | ||
|
@@ -122,7 +123,10 @@ function model(layout, d, i) { | |
line = trace.line, | ||
domain = trace.domain, | ||
dimensions = trace.dimensions, | ||
width = layout.width; | ||
width = layout.width, | ||
labelFont = trace.labelfont, | ||
tickFont = trace.tickfont, | ||
rangeFont = trace.rangefont; | ||
|
||
var lines = Lib.extendDeep({}, line, { | ||
color: lineColor.map(domainToUnitScale({values: lineColor, range: [line.cmin, line.cmax]})), | ||
|
@@ -144,6 +148,9 @@ function model(layout, d, i) { | |
tickDistance: c.tickDistance, | ||
unitToColor: unitToColorScale(cscale), | ||
lines: lines, | ||
labelFont: labelFont, | ||
tickFont: tickFont, | ||
rangeFont: rangeFont, | ||
translateX: domain.x[0] * width, | ||
translateY: layout.height - domain.y[1] * layout.height, | ||
pad: pad, | ||
|
@@ -227,8 +234,6 @@ function styleExtentTexts(selection) { | |
selection | ||
.classed('axisExtentText', true) | ||
.attr('text-anchor', 'middle') | ||
.style('font-weight', 100) | ||
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. @monfera is removing that font-weight setting on purpose? Unsurprisingly, it makes the image tests fail. 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. As we discussed about font standardization... I assumed, perhaps incorrectly, that we don't want to customize it. I do have a preference for the lower font weight as in the case of 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. We don't set This is another thing I wished I would've noticed while reviewing your first parcoords PR. Oh well. I think it's best to drop 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. Cool, in this case I could guess, if a bit late, how you'd go about it :-) |
||
.style('font-size', '10px') | ||
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. Ha. I see I guess we'll have to keep it this way until v2 😬 |
||
.style('cursor', 'default') | ||
.style('user-select', 'none'); | ||
} | ||
|
@@ -556,22 +561,18 @@ module.exports = function(root, svg, styledData, layout, callbacks) { | |
null) | ||
.tickFormat(d.ordinal ? function(d) {return d;} : null) | ||
.scale(scale)); | ||
Drawing.font(axis.selectAll('text'), d.model.tickFont); | ||
}); | ||
|
||
axis | ||
.selectAll('.domain, .tick') | ||
.selectAll('.domain, .tick>line') | ||
.attr('fill', 'none') | ||
.attr('stroke', 'black') | ||
.attr('stroke-opacity', 0.25) | ||
.attr('stroke-width', '1px'); | ||
|
||
axis | ||
.selectAll('text') | ||
.style('font-weight', 100) | ||
.style('font-size', '10px') | ||
.style('fill', 'black') | ||
.style('fill-opacity', 1) | ||
.style('stroke', 'none') | ||
.style('text-shadow', '1px 1px 1px #fff, -1px -1px 1px #fff, 1px -1px 1px #fff, -1px 1px 1px #fff') | ||
.style('cursor', 'default') | ||
.style('user-select', 'none'); | ||
|
@@ -590,15 +591,14 @@ module.exports = function(root, svg, styledData, layout, callbacks) { | |
.append('text') | ||
.classed('axisTitle', true) | ||
.attr('text-anchor', 'middle') | ||
.style('font-family', 'sans-serif') | ||
.style('font-size', '10px') | ||
.style('cursor', 'ew-resize') | ||
.style('user-select', 'none') | ||
.style('pointer-events', 'auto'); | ||
|
||
axisTitle | ||
.attr('transform', 'translate(0,' + -c.axisTitleOffset + ')') | ||
.text(function(d) {return d.label;}); | ||
.text(function(d) {return d.label;}) | ||
.each(function(d) {Drawing.font(axisTitle, d.model.labelFont);}); | ||
|
||
var axisExtent = axisOverlays.selectAll('.axisExtent') | ||
.data(repeat, keyFun); | ||
|
@@ -631,7 +631,8 @@ module.exports = function(root, svg, styledData, layout, callbacks) { | |
.call(styleExtentTexts); | ||
|
||
axisExtentTopText | ||
.text(function(d) {return formatExtreme(d)(d.domainScale.domain().slice(-1)[0]);}); | ||
.text(function(d) {return formatExtreme(d)(d.domainScale.domain().slice(-1)[0]);}) | ||
.each(function(d) {Drawing.font(axisExtentTopText, d.model.rangeFont);}); | ||
|
||
var axisExtentBottom = axisExtent.selectAll('.axisExtentBottom') | ||
.data(repeat, keyFun); | ||
|
@@ -653,7 +654,8 @@ module.exports = function(root, svg, styledData, layout, callbacks) { | |
.call(styleExtentTexts); | ||
|
||
axisExtentBottomText | ||
.text(function(d) {return formatExtreme(d)(d.domainScale.domain()[0]);}); | ||
.text(function(d) {return formatExtreme(d)(d.domainScale.domain()[0]);}) | ||
.each(function(d) {Drawing.font(axisExtentBottomText, d.model.rangeFont);}); | ||
|
||
var axisBrush = axisOverlays.selectAll('.axisBrush') | ||
.data(repeat, keyFun); | ||
|
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.
@monfera this is what I came off with.
As you pointed out, changing the font size default to 12 makes most parcoord graphs look less-than-optimal. We should fix that text-overlapping problem at some point though - similar to how we do it currently in cartesian axes. But that's for another PR. So, the 10px default value remains.
But, to make this a little more plotly-esque, we make the parcoord font size scale linearly with
layout.font
similar to what's done for axestitlefont
.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.
Thanks @etpinard, also for the pointer!