PR10 of ck-diagram: edge selection, label editing, color picker #57

Manually merged
tvidal merged 3 commits from feat/057-pr10-of-ck-diagram-edge-selection-label-editin into master 2026-08-03 09:30:00 +02:00
Owner

PR10 of ck-diagram: edge selection, label editing, color picker

Along the way:

  • PR11 of ck-diagram: file integration via files_coordinator
  • PR12 of ck-diagram: SVG + PNG export

Touches 21 files: Cargo.lock, ck-diagram/Cargo.toml, ck-diagram/src/canvas.rs, ck-diagram/src/export.rs, ck-diagram/src/history.rs, ck-diagram/src/lib.rs and 15 more.

PR10 of ck-diagram: edge selection, label editing, color picker Along the way: - PR11 of ck-diagram: file integration via files_coordinator - PR12 of ck-diagram: SVG + PNG export Touches 21 files: `Cargo.lock`, `ck-diagram/Cargo.toml`, `ck-diagram/src/canvas.rs`, `ck-diagram/src/export.rs`, `ck-diagram/src/history.rs`, `ck-diagram/src/lib.rs` and 15 more.
tvidal approved these changes 2026-08-01 09:30:00 +02:00
tvidal left a comment

A clear improvement. The change in ck-diagram/src/export.rs is the part I would have argued for.

A clear improvement. The change in `ck-diagram/src/export.rs` is the part I would have argued for.
@ -0,0 +575,4 @@
/// Format an `f32` for the SVG attribute stream — three decimals,
/// trailing zeros trimmed, so the output is compact without slipping
/// into scientific notation on extreme values.
fn fmt(v: f32) -> String {
Owner

fmt reads, decides and writes in one function. The deciding part is the piece I would want to test on its own. Worth splitting?

`fmt` reads, decides and writes in one function. The deciding part is the piece I would want to test on its own. Worth splitting?
@ -5091,0 +5246,4 @@
});
view.cycle_edge_routing(&mut diagram);
assert!(matches!(
diagram.edges.iter().find(|e| e.id == edge_id).unwrap().routing,
Owner

iter unwraps here. If this is reachable from user input it is a panic in front of a person rather than an error they can read. Is it provably there at this point, or should it carry a ??

`iter` unwraps here. If this is reachable from user input it is a panic in front of a person rather than an error they can read. Is it provably there at this point, or should it carry a `?`?
@ -0,0 +626,4 @@
let svg = to_svg(&d);
let opts = usvg::Options::default();
usvg::Tree::from_str(&svg, &opts).expect("complex svg parses");
Owner

The expect message on usvg is the only thing a user sees if this fires. Does it say what they should do, or only what we assumed?

The expect message on `usvg` is the only thing a user sees if this fires. Does it say what they should do, or only what we assumed?
Author
Owner

Split. The decision in fmt is a pure function now, with its own tests in ck-diagram/src/export.rs.

Split. The decision in `fmt` is a pure function now, with its own tests in `ck-diagram/src/export.rs`.
Author
Owner

It is reachable. The function in ck-diagram/tests/canvas.rs returns a Result now and the caller reports it, so the unwrap on iter is gone.

It is reachable. The function in `ck-diagram/tests/canvas.rs` returns a Result now and the caller reports it, so the unwrap on `iter` is gone.
Author
Owner

It said what I assumed. Reworded in ck-diagram/tests/export.rs so it names the input that broke it, which is what the bug report will need.

It said what I assumed. Reworded in `ck-diagram/tests/export.rs` so it names the input that broke it, which is what the bug report will need.
ebrandt approved these changes 2026-08-03 09:00:00 +02:00
ebrandt left a comment

Read it too. No objection to ck-diagram/src/export.rs.

Read it too. No objection to `ck-diagram/src/export.rs`.
Edge selection lands as a parallel `selected_edges: Vec<EdgeId>` on
DiagramView with click hit-testing against the rendered polyline
(EDGE_HIT_RADIUS = 5 screen px), shift / ctrl-click semantics matching
shapes, Ctrl+A inclusion, delete with a new HistoryEntry::DeleteEdges
variant, and a wide blue selection chrome over the edge stroke.

Edge style mutators (set_edge_stroke_color / _width / _dashed /
_arrow_start / _arrow_end / _routing + cycle_edge_routing) record
EditEdgeStyle / EditEdgeRouting / EditEdgeLabel entries with full undo
/ redo. Stroke colour and width on the existing toolbar cells now flow
into edges when an edge is selected so the same gesture covers shapes
and edges.

Edge label editing reuses the PR7 overlay machinery. EditingState is
now an enum (Shape | Edge); F2 with a single edge selection opens a
TextEdit over the polyline midpoint via edge_label_edit_id, normalises
empty input to None on commit, and records EditEdgeLabel.

Four new toolbar cells (EdgeRouting, EdgeDashed, EdgeArrowStart,
EdgeArrowEnd) live in their own section, only enable when an edge is
selected, and reflect current edge state via cell_highlighted.

The colour swatches gain a real picker: a secondary-click opens egui's
RGB picker popup anchored under the swatch, applies colour changes as
they happen, and dismisses on Escape / outside-click / Close. The
existing left-click palette cycle stays for muscle memory and to keep
PR8's one-tap workflow intact.

18 new integration tests cover selection, hit-testing, history round
trips, toolbar dispatch, F2 label entry, and picker layout — including
a wider-screen helper because the toolbar grew past 800px with the new
edge section.
Wires `.ckdiagram.json` documents through the existing buffer / save /
watcher / external-change-banner pipeline so diagrams become first-class
files instead of a scratch toggle:

- New `ckeditor::diagram_handle` module bundles `Diagram` + `DiagramView`
  plus a JSON snapshot of what's in the rope. `load` parses on open;
  `refresh_from_rope` re-parses on reload; `sync_into_rope` re-serializes
  every gesture commit through `load_from_string` + `Buffer::mark_dirty`
  (new on `ck-editor`), so save / dirty / watcher flow unchanged through
  `project::save_active` and `files_coordinator`.
- `Buffer.diagram: Option<DiagramHandle>` powers payload-aware render
  dispatch in `app_layout::buffer_panel`. Load canonicalizes the rope to
  diagram's `to_json_pretty()` so the snapshot invariant
  `last_synced_json == rope_contents` holds from frame zero — hand-edited
  files don't open dirty.
- `:diagram-new` command seeds `<worktree>/docs/diagrams/untitled.ckdiagram.json`
  (auto-uniquify suffix on collision) and opens it through the normal
  `open_file` path.
- Retires the scratch `:diagram` toggle: drops
  `LayoutState.show_diagram/diagram/diagram_view`, `fresh_diagram`,
  `SCRATCH_NAME`, and the scratch `show()` entry point on
  `panels::diagram_panel`.

Architecture lint baselines updated for the structural changes (new
`diagram_handle` module, retired scratch toggle) — see
`baseline_CHANGELOG.md`. 14 new unit tests across `buffer.rs`,
`diagram_handle.rs`, and `command_bar.rs`. All four gates green.
Closes the export gap that PR-10-as-originally-planned was scoped for
(the actual landed PR 10 c34891f did edge selection / label editing /
color picker instead, so export had to be done as a follow-on).

- New `ck_diagram::export` module emits a self-contained SVG 1.1
  document from a `Diagram`. Covers all seven shape kinds (rect,
  rounded rect, ellipse, diamond, database cylinder, process,
  text), all three edge routings (straight / orthogonal Z route /
  cubic Bezier), arrowheads via SVG `<marker>` defs, dashed
  strokes, edge labels, NodeText (Plain + TitleBody) with H/V
  alignment, XML escaping, RGBA opacity, and a viewBox tight to
  content with a 16-unit margin. `Diagram::to_svg(&self) -> String`
  is the user-facing entry point.
- New `ckeditor::diagram_export` module rasterises the SVG via
  `usvg` + `resvg` + `tiny_skia`. Loads system fonts so labels
  render in the PNG. 4096-px ceiling per side prevents pathological
  diagrams from blowing up RAM. `ExportFormat::parse`, `export_bytes`,
  `pick_export_path` make up the public surface.
- `:diagram-export svg|png` lands in command_bar via a new
  free-function dispatcher `try_execute_diagram_command` that
  also absorbs `:diagram-new` from PR 11. Folding both diagram
  commands into one dispatcher trimmed `app_layout::CkEditorApp`
  CC from 433 → 432.
- `ck_files::write_atomic_bytes` adds a binary-payload sibling to
  `write_atomic` so PNG bytes go through the same temp + fsync +
  rename seam text writes use.

Tests: 30 new integration tests in `ck-diagram/tests/export.rs`
(usvg parse-back round-trip plus per-feature assertions) + 7 unit
tests in `ckeditor::diagram_export` (format parse, SVG/PNG bytes,
PNG magic header, path picking with collision uniquify). All four
gates green; baseline ratcheted (CC tightened).
tvidal manually merged commit 79cd20b2b6 into master 2026-08-03 09:30:00 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
northgale/ckeditor!57
No description provided.