Repository navigation
Conversation
…nd first 5 chart types Co-Authored-By: Claude Code <noreply@anthropic.com>
FredApps
requested changes
Oct 18, 2026
FredApps
left a comment
Contributor
There was a problem hiding this comment.
- This overlaps the open #255, which already contains the data window, title box, type picker, double-click editing and Insert > Chart opening the editor, for all chart types, with yefan911 credited. Please say how this PR relates to it, or close it in favour of #255. Two editors for the same dialog will conflict in
dialogs.rsandinsert.rs. - The PR mixes separate changes. Please split: (1) the data editor dialog; (2)
chart.titleplus the Chart Design title/element toggles (ribbon.rs:3061-3087); (3) the Format pane "Chart Options" tab (panes.rs:448-483); (4) data labels for Line/Area (render/src/chart.rs:753-770), which is a renderer change. dialogs.rs(chartData): every edit in a cell, the title box and the type buttons runschart.data/chart.title/chart.typeon each change. The old dialog applied once on OK and could be cancelled. Is there one undo step per keystroke now? Typing12.5would be four steps, and Cancel no longer exists. If so, apply on commit (focus lost or Enter) or coalesce. Please add a test that one cell edit is one undo step.ribbon.rs:1243passeseditData: truefor every chart type from Insert > Chart. The grid is categories + series only. Does it make sense for scatter, bubble or stock charts? #255 has per-type columns for this reason. Please test inserting a scatter chart and editing one cell.dialogs.rsbanner hard-codesColor32::from_rgb(0x10, 0x7C, 0x41)and0x70,0x30,0xA0, and the button says "Edit Data in GridCraft" (panes.rs:480) although nothing opens GridCraft. UseTokenscolours and a label that says what happens.panes.rs:480also sendsid_param, whichapp.dialogignores (it falls back to the selection).- The render test
first_five_charts_render_with_data_labelsonly checks that more than 100 pixels are non-white. It passes with labels off. Please compare the render withdata_labelson and off for Line and Area. - Question: where do the new English strings go for localisation? Please confirm deckcraft does not need catalog entries for dialog text; if it does, add them in every language.
- ["cargo","fmt","--all","--","--check"]
- ["cargo","clippy","--workspace","--all-targets","--","-D","warnings"]
- ["cargo","test","-p","deckcraft-engine","chart_first_five_types_and_title_data_flow"] (expect 1 match)
- ["cargo","test","-p","deckcraft-render","first_five_charts_render_with_data_labels"] (expect 1 match)
- ["cargo","test","-p","deckcraft-ui-egui","--lib"] (existing UI tests; none new for the dialog)
- ["cargo","xtask","ci"] (the repo's gate: fmt, clippy, tests, assets, layers, wasm; read .github/workflows/ci.yml for the exact steps)
No existing holds. Note the engine test usesunwrap/panic!(allowed in tests only).
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
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.
Summary
Addresses feedback from #195 to provide a self-contained, in-app spreadsheet data editor for charts with live sync and
full editing support for the first 5 primary chart types (Column, Bar, Line, Area, Pie).
Key Changes
crates/ui-egui/src/dialogs.rs):A (Categories),B,C...) and row indices.📋 Copy Table (TSV)button to copy chart data to clipboard for easy pasting into GridCraft or Excel withoutspawning processes.
crates/engine):chart.gridcraftcommand, and external binary paths from the engine.insert.chartremains quiet; opens data editor only wheneditData: trueis explicitlyrequested.
chartDataeditor dialog.chart.titlecommand and enabled settingtitledirectly viachart.data.crates/model,crates/render):ChartType::FIRST_FIVEconstant.data_labelsis active.crates/ui-egui/src/ribbon.rs,crates/ui-egui/src/panes.rs):CLAUDE.md,AGENTS.md, andCargo.lockremain completely untouched.Tests
crates/engine/src/tests.rs:chart_first_five_types_and_title_data_flowcrates/render/src/tests.rs:first_five_charts_render_with_data_labels🤖 Generated with Claude Code