adding layout.colorway #2
Conversation
src/plots/plots.js
Outdated
| @@ -454,6 +454,9 @@ plots.supplyDefaults = function(gd) { | |||
|
|
|||
| newFullLayout._initialAutoSizeIsDone = true; | |||
|
|
|||
| // pass along trace colors | |||
| newFullLayout._colorway = newLayout.colorway; | |||
There was a problem hiding this comment.
That works, but we can do better. Ideally, we should use Lib.coerce to validate and map colorway from input layout to full layout. layout.colorway is a bit of a special case: we don't have a valType that corresponds to the desired behavior. That said, we should at the very least make sure that colorway is an array and (manually) validate every color in the colorway.
src/plots/layout_attributes.js
Outdated
| @@ -175,5 +175,6 @@ module.exports = { | |||
| role: 'info', | |||
| editType: 'legend', | |||
| description: 'Determines whether or not a legend is drawn.' | |||
| } | |||
| }, | |||
| colorway: colorAttrs.defaults, | |||
There was a problem hiding this comment.
This should be:
colorway: {
valType: 'any', // for now ;)
dflt: colorAttrs.defaults,
role: 'style',
editType: 'plot' // (maybe 'calc')
description: 'describe this thing!'
}
src/plot_api/helpers.js
Outdated
| @@ -192,7 +192,8 @@ function cleanAxRef(container, attr) { | |||
|
|
|||
| // Make a few changes to the data right away | |||
| // before it gets used for anything | |||
| exports.cleanData = function(data, existingData) { | |||
| exports.cleanData = function(data, existingData, colorway) { | |||
There was a problem hiding this comment.
cleanData is intended for legacy code only, so maybe we don't have to propagate colorway here.
|
@cixzhang looking great! Thanks very much for doing this! Let me know if you have any questions. |
|
@etpinard Thanks so much for the suggestions! I've implemented your comments. Let me know if there's anything more I should do! |
|
I noticed that my implementation of the |
|
Great @cixzhang - this is looking pretty nice. Would you mind squashing your commits into one and making a PR to the main plotly.js repo? |
|
Thanks for all your help! I made a new branch with the squashed changes to avoid force pushing: plotly#2156. Since I added a mock, the CI test failed. I'm guessing I'm missing a step to register the new mock. Any advice? |
For plotly#2136
This PR adds the
layout.colorwayattribute which takes a list of colors to use as the default set of colors for all traces. By default, it should use the existingColor.defaults.