northgale/ckeditor
Watch
12
0
Fork
You've already forked ckeditor
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Wire :diagram into the CkEditor shell #54

Manually merged
tvidal merged 4 commits from feat/054-wire-diagram-into-the-ckeditor-shell into master 2026-07-06 12:49:51 +02:00
Conversation 4 Commits 4 Files changed 23 +3003 -239
praghunathan commented 2026-07-03 09:00:00 +02:00
Owner
Copy link

Wire :diagram into the CkEditor shell

Along the way:

  • PR3 of ck-diagram: toolbox + click/drag insert
  • PR4 of ck-diagram: move, resize, delete, undo/redo
  • Plumb runtime_dir through McpBus::spawn / ensure_bus

Touches 23 files: Cargo.lock, ck-diagram/README.md, ck-diagram/src/canvas.rs, ck-diagram/src/history.rs, ck-diagram/src/lib.rs, ck-diagram/src/toolbox.rs and 17 more.

Wire `:diagram` into the CkEditor shell Along the way: - PR3 of ck-diagram: toolbox + click/drag insert - PR4 of ck-diagram: move, resize, delete, undo/redo - Plumb runtime_dir through McpBus::spawn / ensure_bus Touches 23 files: `Cargo.lock`, `ck-diagram/README.md`, `ck-diagram/src/canvas.rs`, `ck-diagram/src/history.rs`, `ck-diagram/src/lib.rs`, `ck-diagram/src/toolbox.rs` and 17 more.
tvidal approved these changes 2026-07-04 11:32:27 +02:00
tvidal left a comment
Copy link

Approving. I would have split ck-diagram/src/history.rs differently, but not enough to hold this up.

Approving. I would have split `ck-diagram/src/history.rs` differently, but not enough to hold this up.
ck-diagram/src/history.rs
@ -0,0 +186,4 @@
/// diagram (the canvas mutates the live shape during the drag);
/// `record` just pushes the entry onto the undo stack and clears
/// the redo stack.
pub fn record(&mut self, entry: HistoryEntry) {
tvidal commented 2026-07-04 11:32:27 +02:00
Owner
Copy link

record is public. Does anything outside the crate call it, or is it pub because that was easier? Every public item is a promise.

`record` is public. Does anything outside the crate call it, or is it pub because that was easier? Every public item is a promise.
ck-diagram/tests/toolbox.rs
@ -0,0 +217,4 @@
}
#[test]
fn click_inserts_above_existing_shapes_in_z() {
tvidal commented 2026-07-04 11:32:27 +02:00
Owner
Copy link

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

`click_inserts_above_existing_shapes_in_z` reads, decides and writes in one function. The deciding part is the piece I would want to test on its own. Worth splitting?
praghunathan commented 2026-07-05 06:30:00 +02:00
Author
Owner
Copy link

Nothing outside calls record. It is pub(crate) in ck-diagram/src/history.rs now.

Nothing outside calls `record`. It is pub(crate) in `ck-diagram/src/history.rs` now.
praghunathan commented 2026-07-05 06:30:00 +02:00
Author
Owner
Copy link

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

Split. The decision in `click_inserts_above_existing_shapes_in_z` is a pure function now, with its own tests in `ck-diagram/tests/toolbox.rs`.
hkowalski approved these changes 2026-07-06 08:00:00 +02:00
hkowalski left a comment
Copy link

Looked through ck-diagram/src/history.rs as well. Fine by me.

Looked through `ck-diagram/src/history.rs` as well. Fine by me.
The diagramming PRs that follow PR 2 (toolbox, move/resize, edges,
text editing, style toolbar, minimap, export) all build on
`ck_diagram::DiagramView` but stay invisible from the running app
until PR 11 wires `.ckdiagram.json` files through
`files_coordinator`. Add a thin shell entry point so each subsequent
PR is exercisable in the running app instead of library-only.

`:diagram` toggles a scratch `DiagramView` in the buffer area,
backed by an in-memory `Diagram` held on `LayoutState`. PR 11 will
replace this with per-buffer file-backed diagrams; until then the
single shared scratch diagram is enough to validate each canvas
feature as it lands. The new host module
`ckeditor/src/panels/diagram_panel.rs` exports `fresh_diagram`
(used by `LayoutState::default`) and a `show` free function that
takes `&Theme` so CK005 stays satisfied even though the theme is
not yet visually load-bearing.

The two new branches (`if self.layout.show_diagram` in
`render_buffer`, the `CMD_DIAGRAM` arm in
`execute_panel_command`) would have raised
`app_layout::CkEditorApp` cyclomatic from 440 to 442. Offset by
extracting the trailing `||`-chain in `try_execute_tree_command`
to a free function `is_tree_command_prefix` — same pattern as
`dashboard_keyboard_focused` and `show_active_buffer_if_present`.
Net baseline shift: CC 440→438, LCOM1 355→337, RFC 251→250.
Dated entry in `ckeditor-lint/baseline_CHANGELOG.md`.
Adds a left-side toolbox (Basic + Flowchart sections per design-doc
§6.2) that inserts shapes onto the canvas — click an entry to place
at the viewport center, drag an entry onto the canvas to place at the
cursor's world position. The new shape uses DEFAULT_INSERT_SIZE
(120 × 70), ShapeStyle::default(), a kind-named text label, takes
z = max + 1 so it paints on top, and becomes the current selection.

DiagramView::show now orchestrates a SidePanel(toolbox) + canvas
split; the canvas body lives in a new private show_canvas helper
that returns the response and live transform so the drop branch can
recover the cursor's world position via dnd_release_payload. Toolbox
buttons use deterministic egui Ids (toolbox_entry_id) so headless
integration tests can locate each entry's screen rect via
ctx.read_response without depending on SidePanel inner-margin or
font metrics.

Coverage: 100% lines / regions / functions across the whole crate.
Lint baseline unchanged (ck-diagram is outside ckeditor-lint scope).
Drag a selected shape's body to move it; drag any of eight handles
around the selection chrome to resize it (clamped to MIN_SHAPE_SIZE
world units, opposite edge held fixed). Delete / Backspace remove the
selection. Ctrl+Z / Ctrl+Y / Ctrl+Shift+Z walk a command-shaped
History stack (Insert / Move / Resize / Delete) — one entry per
gesture, recorded on mouseup so mid-drag previews stay smooth and
the undo log stays usable.

History lives on DiagramView (UI state, not persisted), so toggling
:diagram off and back on preserves both the diagram and the undo
stack. Snap-to-grid + Alt-bypass deferred to a follow-up slice.
Coverage stays at 100% lines / regions / functions across the crate;
mutation gate stays off per the iteration-mode contract until the
end-of-feature seal pass.
Tests used to mutate `$XDG_RUNTIME_DIR` via `unsafe std::env::set_var`
through a `RuntimeDirGuard`, which raced with parallel scenarios in
the same `cargo test` binary and intermittently tripped the 500 ms
`BIND_TIMEOUT` in `scenario_ensure_bus_is_idempotent`. The fix passes
the runtime dir in by value: `McpBus::spawn` and `mcp_host::ensure_bus`
each take a `PathBuf`, production resolves it once via the new
`mcp_host::default_runtime_dir()`, and tests pass their own tempdir
straight through. Every `RuntimeDirGuard` is deleted.

5x repeats of the previously flaky test all green.
tvidal manually merged commit ba73fc0a01 into master 2026-07-06 12:49:51 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
tvidal
hkowalski
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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!54
No description provided.