Skip to content

adding layout.colorway #2156

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

Merged
merged 4 commits into from
Nov 15, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
16 changes: 16 additions & 0 deletions src/lib/coerce.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,22 @@ exports.valObjectMeta = {
else propOut.set(dflt);
}
},
colorlist: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind adding a Lib.validate test case for the new colorlist valType?

description: [
'A list of colors.',
'Must be an {array} containing valid colors.',
].join(' '),
requiredOpts: [],
otherOpts: ['dflt'],
coerceFunction: function(v, propOut, dflt) {
function isColor(color) {
return tinycolor(color).isValid();
}
if(!Array.isArray(v)) propOut.set(dflt);
else if(v.every(isColor)) propOut.set(v);
else propOut.set(dflt);
}
},
colorscale: {
description: [
'A Plotly colorscale either picked by a name:',
Expand Down
9 changes: 8 additions & 1 deletion src/plots/layout_attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,5 +175,12 @@ module.exports = {
role: 'info',
editType: 'legend',
description: 'Determines whether or not a legend is drawn.'
}
},
colorway: {
valType: 'colorlist',
dflt: colorAttrs.defaults,
role: 'style',
editType: 'calc',
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment for future development: note here that only pie traces require this to be editType: 'calc'. For most trace types, editType: 'plot' would have sufficed and even editType: 'style' might have been good enough for cartesian traces. Oh well, we might have to add-trace-type-specific editType flags for layout attributes down the road to improve performance.

description: 'Sets the default trace colors.'
},
};
6 changes: 5 additions & 1 deletion src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -950,8 +950,9 @@ plots.supplyFrameDefaults = function(frameIn) {
};

plots.supplyTraceDefaults = function(traceIn, traceOutIndex, layout, traceInIndex) {
var colorway = layout.colorway || Color.defaults;
var traceOut = {},
defaultColor = Color.defaults[traceOutIndex % Color.defaults.length];
defaultColor = colorway[traceOutIndex % colorway.length];

function coerce(attr, dflt) {
return Lib.coerce(traceIn, traceOut, plots.attributes, attr, dflt);
Expand Down Expand Up @@ -1138,6 +1139,8 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut) {
coerce('separators');
coerce('hidesources');

coerce('colorway');

Registry.getComponentMethod(
'calendars',
'handleDefaults'
Expand Down Expand Up @@ -2171,6 +2174,7 @@ plots.doCalcdata = function(gd, traces) {

// for sharing colors across pies (and for legend)
fullLayout._piecolormap = {};
fullLayout._piecolorway = null;
fullLayout._piedefaultcolorcount = 0;

// If traces were specified and this trace was not included,
Expand Down
37 changes: 26 additions & 11 deletions src/traces/pie/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,18 @@ module.exports = function calc(gd, trace) {
var colors = trace.marker.colors;
var cd = [];
var fullLayout = gd._fullLayout;
var colorWay = fullLayout.colorway;
var colorMap = fullLayout._piecolormap;
var allThisTraceLabels = {};
var vTotal = 0;
var hiddenLabels = fullLayout.hiddenlabels || [];

var i, v, label, hidden, pt;

if(!fullLayout._piecolorway && colorWay !== Color.defaults) {
fullLayout._piecolorway = generateDefaultColors(colorWay);
}

if(trace.dlabel) {
labels = new Array(vals.length);
for(i = 0; i < vals.length; i++) {
Expand Down Expand Up @@ -107,7 +112,10 @@ module.exports = function calc(gd, trace) {
pt.color = colorMap[pt.label];
}
else {
colorMap[pt.label] = pt.color = nextDefaultColor(fullLayout._piedefaultcolorcount);
colorMap[pt.label] = pt.color = nextDefaultColor(
fullLayout._piedefaultcolorcount,
fullLayout._piecolorway
);
fullLayout._piedefaultcolorcount++;
}
}
Expand Down Expand Up @@ -148,22 +156,29 @@ module.exports = function calc(gd, trace) {
*/
var pieDefaultColors;

function nextDefaultColor(index) {
function nextDefaultColor(index, pieColorWay) {
if(!pieDefaultColors) {
// generate this default set on demand (but then it gets saved in the module)
var mainDefaults = Color.defaults;
pieDefaultColors = mainDefaults.slice();
pieDefaultColors = generateDefaultColors(mainDefaults);
}

var i;
var pieColors = pieColorWay || pieDefaultColors;
return pieColors[index % pieColors.length];
}

for(i = 0; i < mainDefaults.length; i++) {
pieDefaultColors.push(tinycolor(mainDefaults[i]).lighten(20).toHexString());
}
function generateDefaultColors(colorList) {
var i;

for(i = 0; i < Color.defaults.length; i++) {
pieDefaultColors.push(tinycolor(mainDefaults[i]).darken(20).toHexString());
}
var pieColors = colorList.slice();

for(i = 0; i < colorList.length; i++) {
pieColors.push(tinycolor(colorList[i]).lighten(20).toHexString());
}

for(i = 0; i < colorList.length; i++) {
pieColors.push(tinycolor(colorList[i]).darken(20).toHexString());
}

return pieDefaultColors[index % pieDefaultColors.length];
return pieColors;
}
32 changes: 32 additions & 0 deletions test/image/mocks/layout-colorway.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"data": [
{"y": [8, 7, 7, 6, 5, 4, 4, 3, 2, 2, 3]},
{"y": [7, 7, 6, 5, 4, 4, 3, 2, 2, 3, 8]},
{"y": [7, 6, 5, 4, 4, 3, 2, 2, 3, 8, 7]},
{"y": [6, 5, 4, 4, 3, 2, 2, 3, 8, 7, 7]},
{"y": [5, 4, 4, 3, 2, 2, 3, 8, 7, 7, 6]},
{
"labels": ["a","b","c","c","c","a","d","e","f","f","g","h"],
"type": "pie",
"domain": {"x": [0, 0.4]},
"xaxis": "x2",
Copy link
Contributor

Choose a reason for hiding this comment

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

xaxis and yaxis don't do anything for pie traces. Would you mind 🔪 them?

"yaxis": "y2"
}
],
"layout": {
"title": "Custom Trace Color Defaults",
"colorway": [
"#DE5845",
"#E83898",
"#A83DD1",
"#5A38E8",
"#3C71DE"
],
"xaxis": {
"domain": [0.4, 1]
},
"yaxis": {
"anchor": "y2"
Copy link
Contributor

Choose a reason for hiding this comment

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

anchor for y-axes expects x-axis ids, so this thing here gets ignored. 🔪

}
}
}
41 changes: 41 additions & 0 deletions test/jasmine/tests/pie_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,44 @@ describe('Test event data of interactions on a pie plot:', function() {
});
});
});

describe('pie relayout', function() {
var gd;

beforeEach(function() { gd = createGraphDiv(); });

afterEach(destroyGraphDiv);

it('will update colors when colorway is updated', function(done) {
var originalColors = [
'rgb(255,0,0)',
'rgb(0,255,0)',
'rgb(0,0,255)',
];

var relayoutColors = [
'rgb(255,255,0)',
'rgb(0,255,255)',
'rgb(255,0,255)',
];

function checkRelayoutColor(d, i) {
expect(this.style.fill.replace(/\s/g, '')).toBe(relayoutColors[i]);
}

Plotly.newPlot(gd, [{
labels: ['a', 'b', 'c', 'a', 'b', 'a'],
type: 'pie'
}], {
colorway: originalColors
})
.then(function() {
return Plotly.relayout(gd, 'colorway', relayoutColors);
})
.then(function() {
var slices = d3.selectAll('.slice path');
slices.each(checkRelayoutColor);
})
.then(done);
});
});