Skip to content

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.

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 6 commits into from
May 9, 2017
Merged

Parcoords fonts #1624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
505614d
parcoords fonts
monfera Apr 26, 2017
9939241
specify default sizes for parcoords fonts; screenshot fix
monfera Apr 26, 2017
a8e444a
avoid duplicate coercion
monfera Apr 26, 2017
3925d6d
make parcoord font size dflt scale linearly with global font
etpinard May 1, 2017
a3544c7
showcase new parcoord font attributes in `parcoords_2` mock
etpinard May 1, 2017
c0e0845
update parcoords baselines (:hocho: font-weight)
etpinard May 1, 2017
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
11 changes: 11 additions & 0 deletions src/traces/parcoords/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ var colorAttributes = require('../../components/colorscale/color_attributes');
var colorbarAttrs = require('../../components/colorbar/attributes');
var colorscales = require('../../components/colorscale/scales');
var axesAttrs = require('../../plots/cartesian/layout_attributes');
var fontAttrs = require('../../plots/font_attributes');

var extendDeep = require('../../lib/extend').extendDeep;
var extendFlat = require('../../lib/extend').extendFlat;
Expand Down Expand Up @@ -47,6 +48,16 @@ module.exports = {
}
},

labelfont: extendFlat({}, fontAttrs, {
description: 'Sets the font for the `dimension` labels.'
}),
tickfont: extendFlat({}, fontAttrs, {
description: 'Sets the font for the `dimension` tick values.'
}),
rangefont: extendFlat({}, fontAttrs, {
description: 'Sets the font for the `dimension` range values.'
}),

dimensions: {
_isLinkedToArray: 'dimension',
label: {
Expand Down
13 changes: 12 additions & 1 deletion src/traces/parcoords/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ function dimensionsDefaults(traceIn, traceOut) {
return dimensionsOut;
}


module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
function coerce(attr, dflt) {
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
Expand All @@ -97,4 +96,16 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
if(!Array.isArray(dimensions) || !dimensions.length) {
traceOut.visible = false;
}

// make default font size 10px,
// scale linearly with global font size
var fontDflt = {
family: layout.font.family,
size: Math.round(layout.font.size * (10 / 12)),
Copy link
Contributor

@etpinard etpinard May 1, 2017 •
edited
Loading

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 axes titlefont.

Copy link
Contributor Author

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!

color: layout.font.color
};

Lib.coerceFont(coerce, 'labelfont', fontDflt);
Lib.coerceFont(coerce, 'tickfont', fontDflt);
Lib.coerceFont(coerce, 'rangefont', fontDflt);
};
30 changes: 16 additions & 14 deletions src/traces/parcoords/parcoords.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;}
Expand Down Expand Up @@ -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]})),
Expand All @@ -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,
Expand Down Expand Up @@ -227,8 +234,6 @@ function styleExtentTexts(selection) {
selection
.classed('axisExtentText', true)
.attr('text-anchor', 'middle')
.style('font-weight', 100)
Copy link
Contributor

@etpinard etpinard May 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 parcoords the shape of line distribution is more important than the best readability so it's fine to keep the weight=100 but with configuration, the user can probably control the font anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't set font-weight anyway in our code - though inputting text as <b>text</b> effectively does that.

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 font-weight and make parcoords look a little more plotly-esque.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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')
Copy link
Contributor

Choose a reason for hiding this comment

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

Ha. I see 10px was used before. I should've caught that. Elsewhere in plotly.js this would be 12px.

I guess we'll have to keep it this way until v2 😬

.style('cursor', 'default')
.style('user-select', 'none');
}
Expand Down Expand Up @@ -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');
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Binary file modified test/image/baselines/gl2d_parcoords.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl2d_parcoords_1.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl2d_parcoords_2.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl2d_parcoords_blocks.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl2d_parcoords_large.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 19 additions & 2 deletions test/image/mocks/gl2d_parcoords_2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
{
"layout": {
"width": 500,
"height": 500
"height": 500,
"font": {
"size": 15
}
},

"data": [{
Expand All @@ -13,13 +16,27 @@
"y": [0.2, 0.7]
},

"labelfont": {
"color": "red",
"size": 20
},
"tickfont": {
"color": "blue"
},
"rangefont": {
"color": "green"
},

"line": {
"showscale": true,
"reversescale": true,
"colorscale": "Jet",
"cmin": -4000,
"cmax": -100,
"color": [-41, -1317, -164, -1856, -79, -931, -191, -2983, -341, -3846]
"color": [-41, -1317, -164, -1856, -79, -931, -191, -2983, -341, -3846],
"colorbar": {
"x": 1.1
}
},

"dimensions": [
Expand Down
22 changes: 21 additions & 1 deletion test/jasmine/tests/parcoords_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,34 @@ describe('parcoords initialization tests', function() {
expect(gd._fullData[0].opacity).toBeUndefined();
});

it('should use global font as label, tick and range font defaults', function() {
var gd = Lib.extendDeep({}, mock1);
gd.layout.font = {
family: 'Gravitas',
size: 20,
color: 'blue'
};

Plots.supplyDefaults(gd);

var expected = {
family: 'Gravitas',
size: 17,
color: 'blue'
};

expect(gd._fullData[0].labelfont).toEqual(expected);
expect(gd._fullData[0].tickfont).toEqual(expected);
expect(gd._fullData[0].rangefont).toEqual(expected);
});
});

describe('parcoords defaults', function() {

function _supply(traceIn) {
var traceOut = { visible: true },
defaultColor = '#444',
layout = { };
layout = { font: Plots.layoutAttributes.font };

Parcoords.supplyDefaults(traceIn, traceOut, defaultColor, layout);

Expand Down