feat(chart): chart-view redesign — config bar, autoChart, 5 types via Chart.js#20
Open
BorisTyshkevich wants to merge 3 commits into
Open
feat(chart): chart-view redesign — config bar, autoChart, 5 types via Chart.js#20BorisTyshkevich wants to merge 3 commits into
BorisTyshkevich wants to merge 3 commits into
Conversation
…s via Chart.js Replace the single hard-wired bar chart with a real chart surface matching the Claude Design handoff: a config bar (Type / X / Y / multi-series toggle / group-by Series), an autoChart heuristic that classifies columns from their ClickHouse types (temporal → line, categorical → horizontal bar, ordinal → column), and five renderers (h-bar / column / line / area / pie) with multi-series and group-by pivot, themed to the CSS tokens. Rendering now uses Chart.js — the one bundled runtime dependency, inlined into dist/sql.html so the page still makes zero third-party requests. This relaxes CLAUDE.md hard rule #4 (recorded there, in the README, and build.mjs). To keep the coverage model intact, all role/axis/pivot/scale math and the Chart.js config builder are pure in src/core/chart-data.js (100%), and the `new Chart()` call is an injected seam (app.Chart) so the DOM wrapper stays fully tested. Chart config persists per query tab (tab.chartCfg/chartKey), re-derived on schema change; live Chart instances are destroyed on re-render to avoid leaks. npm test: 488 passed, per-file gate green. Build: 313 KB self-contained artifact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wtzg34E1iGzKm11TArRpko
BorisTyshkevich
force-pushed
the
feat/chart-views
branch
from
201218a to
2c2dc45
Compare
…t, tighten "All measures" - Show "first 500 of N rows" in the config bar when a result exceeds the chart's CHART_ROW_CAP, so the truncation is never silent (the table still shows all). - chartRole reuses format.js:isNumericType on the stripped type instead of a second copy of the numeric regex. - "All measures" now targets only true measures and excludes the current X column, so it can't plot an ordinal axis (e.g. month) against itself. - Comment chartNumFmt's deliberate divergence from formatRows. (Reviewer's sort-on-every-config-click note left as-is: sortRows no-ops without an active sort, and caching only the chart's sort would diverge from the table.) npm test: 490 passed, per-file gate green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wtzg34E1iGzKm11TArRpko
`.res-body` is display:block, so `.chart-view { flex: 1 }` was inert: the view
collapsed to the config bar's height and Chart.js sized the canvas to a few
pixels — a config bar with no visible chart. Use `height: 100%` (the block
parent has a definite height) for `.chart-view` and `.chart-empty` so the
canvas wrapper gets real height.
Not caught earlier: happy-dom does no layout, and the manual browser harness
attached the canvas into a pre-sized container, bypassing the real `.res-body`
parent. Verified live against an otel system.query_log result.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wtzg34E1iGzKm11TArRpko
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces the single hard-wired bar chart with a real chart surface, matching the Claude Design handoff.
autoChartheuristic — classifies each column from its ClickHouse type (strippingNullable/LowCardinality): temporal X → line, categorical X → horizontal bar, ordinal X → column. Non-chartable results show a hint instead of a broken axis.K/M+YYYY-MMlabels).Dependency decision
Rendering now uses Chart.js — the one bundled runtime dependency, inlined into
dist/sql.html, so the page still makes zero third-party requests. This relaxes CLAUDE.md hard rule #4 (no runtime deps); the change is recorded inCLAUDE.md,README.md, andbuild/build.mjs. React-only libs (Recharts/visx) were ruled out — the app is framework-free.Keeping the layers honest
All role/axis/pivot/scale math and the Chart.js config builder are pure in
src/core/chart-data.js(100/100/100/100); thenew Chart()call is an injected seam (app.Chart, like the fetch/crypto seams), soresults.jsstays fully tested. Net: coverage never had to drop — only the no-deps rule did. Live Chart instances are destroyed on re-render to avoid canvas/observer leaks.Verification
npm test→ 488 passed, per-file coverage gate green.npm run build→ 313 KB self-containeddist/sql.html(Chart.js inlined).🤖 Generated with Claude Code