fix(chart) :: align stacked series on their X values - #1370
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
68fda3f to
91e6ce6
Compare
|
The What I'd need is a clear description of the difference between the old and the new algorithms and why you think this difference is not going to break any reasonable existing chart. |
91e6ce6 to
025b80b
Compare
025b80b to
8b7eb16
Compare
|
I really don't have context on the history of how this function came to be, so I had Claude look into old issues to find out. I've added those examples as tests so that we can check for regression. lmk if you think I may have missed any issues.
But to put it in words :: the algorithm is functionally the same; the new part only adds stacked line, area and bar charts whose x is a date or a number. // before
} else if (categories && chart_type === "bar" && series.length > 1)
// after
} else if (series.length > 1 && (is_stacked || (categories && chart_type === "bar")))Other small changes:
@lovasoa lmk if that makes sense :) |
|
Thank you ! |
|
no prob! thanks for the feedback and review :) |
Addresses #1362
Motivation
xvalue, so a series missing a point is squashed on top of the wrong one.xvalues, but only forbarcharts on a text axis.Description
tests/js/, run by Node's test runner fromnpm test, since this logic is worth testing without a browser.stackedparameter, and add the stacked area example from stacked area graphs / stacked line graphs #1362.Testing
xvalues.npm testchecks 41 files and reports nothing.no console errors on card pagefails for the reason described in fix(biome.js) :: fix remaining lint issues #1366, on this branch and on the unmodified base alike.GitHub cannot base a pull request on a branch that lives in a fork, so all eight target
mainand each one carries the commits of those above it. Review and merge them in order: