Pie colors fix & enhancements #2870
Conversation
- fix inheritance of explicit colors by later traces - allow inheritance (of explicit colors) by earlier traces too - add `piecolorway` and `extendpiecolors` for more control over pie colors
and simplify by moving more stuff into doCalcdata
src/plots/plots.js
Outdated
| @@ -2561,9 +2551,10 @@ plots.doCalcdata = function(gd, traces) { | |||
| for(i = 0; i < fullData.length; i++) calci(i, true); | |||
| for(i = 0; i < fullData.length; i++) calci(i, false); | |||
|
|
|||
| for(i = 0; i < calcInteractionsFuncs.length; i++) calcInteractionsFuncs[i](gd, calcdata); | |||
| plots.doCrossTraceCalc(gd); | |||
There was a problem hiding this comment.
Hmm, I guess we don't need to export doCrossTraceCalc anymore since it's only used here...
There was a problem hiding this comment.
stopped exporting -> 0bf82c8
| @@ -775,7 +775,7 @@ describe('annotations autorange', function() { | |||
| }); | |||
| }) | |||
| .then(function() { | |||
| _assert('auto rng / big tx', [-0.22, 3.57], [0.84, 3.365]); | |||
| _assert('auto rng / big tx', [-0.22, 3.59], [0.84, 3.365]); | |||
There was a problem hiding this comment.
Doesn't matter for CI, and has nothing to do with this PR, but this change makes this test pass on my Mac.
| }; | ||
|
|
||
| // call Bar.crossTraceCalc | ||
| Bar.crossTraceCalc(gd, plotinfo); |
There was a problem hiding this comment.
doCalcdata now, but in fact the test FAILS if we leave this in. That means Bar.crossTraceCalc is not idempotent, though I didn't really look into what it's doing. I don't think this is particularly a problem, but it might indicate it's more fragile than it should be.
There was a problem hiding this comment.
That means Bar.crossTraceCalc is not idempotent, though I didn't really look into what it's doing.
Yep, I noticed that last week. This commit dfada6a helps, but there are more cases where Bar.setPositions oops I mean Bar.crossTraceCalc mutates gd.calcdata.
| .then(done); | ||
| }); | ||
|
|
||
| it('can use a separate pie colorway and disable extended colors', function(done) { |
|
Very nice. Merging the old As an aside, I think it might be best at some point to move the 💃 |
Excellent idea. In general it needs to be in |
Reboot of #2867
Original commit d50334e:
calcInteractionsstep immediately aftercalc.layout.piecolorwayandlayout.extendpiecolorsfor more control over pie colors (cc @nicolaskruchten @VeraZab)Update after #2868, commit ded6e94:
calcInteractionsand usescrossTraceCalcfor piescrossTraceCalcto work with pie (and other subplot-less traces)doCalcdata. In particular, it doesn't need to happen in any other places besidesdoCalcdata, andErrorbars.calccan move in here too since it just needed to happen aftercrossTraceCalc.