Skip to content

Charts: in-app spreadsheet data editor and live editing for first 5 chart types - #950

Closed
yefan911 wants to merge 1 commit into
storytold:mainfrom
yefan911:feature/chart-data-editor-v2
Closed

yefan911 wants to merge 1 commit into
storytold:mainfrom
yefan911:feature/chart-data-editor-v2

Conversation

@yefan911

Copy link
Copy Markdown

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

  • In-App Spreadsheet Window (crates/ui-egui/src/dialogs.rs):
    • Resizable floating dialog with GridCraft-inspired styling and live chart sync.
    • Live cell editing with spreadsheet column headers (A (Categories), B, C...) and row indices.
    • Controls to add/remove categories and series.
    • 📋 Copy Table (TSV) button to copy chart data to clipboard for easy pasting into GridCraft or Excel without
      spawning processes.
    • Inline Chart Title editor and 1-click Chart Type switcher for the first 5 chart types.
  • Engine Layer (crates/engine):
    • Removed all process-spawning logic, chart.gridcraft command, and external binary paths from the engine.
    • Programmatic insert.chart remains quiet; opens data editor only when editData: true is explicitly
      requested.
    • Double-clicking a chart shape on the canvas opens the chartData editor dialog.
    • Added chart.title command and enabled setting title directly via chart.data.
  • Model & Rendering (crates/model, crates/render):
    • Added ChartType::FIRST_FIVE constant.
    • Added data label rendering for Line and Area charts when data_labels is active.
  • Ribbon & Format Pane (crates/ui-egui/src/ribbon.rs, crates/ui-egui/src/panes.rs):
    • Added title editing and element toggles (Data Labels, Gridlines, Legend) in the Chart Design ribbon tab.
    • Added a dedicated Chart Options tab to the Format Shape pane with type picker and element toggles.
  • Repository Integrity:
    • CLAUDE.md, AGENTS.md, and Cargo.lock remain completely untouched.

Tests

  • crates/engine/src/tests.rs: chart_first_five_types_and_title_data_flow
  • crates/render/src/tests.rs: first_five_charts_render_with_data_labels

🤖 Generated with Claude Code

…nd first 5 chart types

Co-Authored-By: Claude Code <noreply@anthropic.com>

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 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.rs and insert.rs.
  • The PR mixes separate changes. Please split: (1) the data editor dialog; (2) chart.title plus 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 runs chart.data / chart.title / chart.type on each change. The old dialog applied once on OK and could be cancelled. Is there one undo step per keystroke now? Typing 12.5 would 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:1243 passes editData: true for 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.rs banner hard-codes Color32::from_rgb(0x10, 0x7C, 0x41) and 0x70,0x30,0xA0, and the button says "Edit Data in GridCraft" (panes.rs:480) although nothing opens GridCraft. Use Tokens colours and a label that says what happens. panes.rs:480 also sends id_param, which app.dialog ignores (it falls back to the selection).
  • The render test first_five_charts_render_with_data_labels only checks that more than 100 pixels are non-white. It passes with labels off. Please compare the render with data_labels on 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 uses unwrap/panic! (allowed in tests only).

@yefan911 yefan911 closed this Oct 19, 2026
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