Skip to content

fix(chart) :: align stacked series on their X values - #1370

Merged
lovasoa merged 1 commit into
sqlpage:mainfrom
81reap:stack/5-chart-stacked
Aug 12, 2026
Merged

fix(chart) :: align stacked series on their X values#1370
lovasoa merged 1 commit into
sqlpage:mainfrom
81reap:stack/5-chart-stacked

Conversation

@81reap

@81reap 81reap commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Addresses #1362

Motivation

  • ApexCharts stacks series by point order, not by x value, so a series missing a point is squashed on top of the wrong one.
  • The chart then incorrect data and totals without any warning.
  • SQLPage already aligned series on their x values, but only for bar charts on a text axis.

Description

  • Order the merged axis by what the series already agree on, falling back to ascending order where they diverge, so one unsorted series cannot reorder the chart.
  • Align whenever a chart is stacked, on top of the existing bar-on-a-text-axis case.
  • Add tests/js/, run by Node's test runner from npm test, since this logic is worth testing without a browser.
  • Document the behaviour on the stacked parameter, and add the stacked area example from stacked area graphs / stacked line graphs #1362.
  • Add a changelog entry.

Testing

  • 13 unit tests cover the merge order, dates compared by value, a null treated as missing, and the input left unmutated.
  • The browser suite passes 33 tests, five of them new, including a stacked area chart whose series do not share their x values.
  • npm test checks 41 files and reports nothing.
  • no console errors on card page fails 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 main and each one carries the commits of those above it. Review and merge them in order:

  1. fix(biome.js) :: fix remaining lint issues #1366 :: fix(biome.js) :: fix remaining lint issues
  2. fix(modal) :: give modal component an accessible name #1367 :: fix(modal) :: give modal component an accessible name
  3. fix(map) :: ignore map coordinates that are not a pair of numbers #1368 :: fix(map) :: ignore map coordinates that are not a pair of numbers
  4. feat(chart) :: render column charts as bar charts #1369 :: feat(chart) :: render column charts as bar charts
  5. fix(chart) :: align stacked series on their X values #1370 :: fix(chart) :: align stacked series on their X values ← this PR
  6. fix(chart) :: line series up on a category axis for every chart type #1371 :: fix(chart) :: line series up on a category axis for every chart type
  7. fix(npm) :: install dependencies only once at root level #1372 :: fix(npm) :: install dependencies only once at root level
  8. feat(typescript) :: typecheck browser JavaScript in CI #1373 :: feat(typescript) :: typecheck browser JavaScript in CI

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Comment thread build.rs Outdated
@lovasoa

lovasoa commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

The align_categories function we have is the result of multiple iterations on user feedback. I'm not confident merging this without a good argument that this is not going to break people's existing charts.

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.

@81reap
81reap force-pushed the stack/5-chart-stacked branch from 91e6ce6 to 025b80b Compare August 11, 2026 23:35
@81reap
81reap force-pushed the stack/5-chart-stacked branch from 025b80b to 8b7eb16 Compare August 12, 2026 00:26
@81reap

81reap commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

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.

commit issue rule it added
5ff3b50 #727 every series gets a point in every category, and skipped categories count as 0
ca00eaa #930 a chart with one series keeps the order the query returned
ee156d9 #951 categories the series don't share are sorted by name
32a875d same rules, faster loop

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:

  • Points keep their z value instead of being rebuilt as {x, y}, so nothing visible changes but we do less work. This also means SQLPage should now be able to handle 3D charts but I think that's better scoped for another PR to properly test and fix any other straggling issues in the codebase.
  • Two equal dates now match. The old code only ran on strings so it wasn't caught before.

@lovasoa lmk if that makes sense :)

@81reap
81reap requested a review from lovasoa August 12, 2026 00:51
@81reap
81reap marked this pull request as ready for review August 12, 2026 00:53
@lovasoa
lovasoa merged commit f603aa8 into sqlpage:main Aug 12, 2026
51 checks passed
@lovasoa

lovasoa commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Thank you !

@81reap
81reap deleted the stack/5-chart-stacked branch August 12, 2026 14:38
@81reap

81reap commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

no prob! thanks for the feedback and review :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants