sorokin.oleg.edu/ARCADIA-Studio
1
0
Fork 0
Code Issues 37 Pull Requests 2 Packages Projects Releases 9 Wiki Activity

fix: keep saved diagram viewport stable across group selection

Browse Source
This commit is contained in:
sorokin.oleg.edu
2026-09-23 01:53:11 -07:00
parent 12a7352dde
commit f6a6010bae
3 changed files with 351 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/oa-desktop/src/app/diagram_focus_tests.rs
+96 -3
View File
@@ -154,9 +154,6 @@ async fn mounted_f2_requires_canvas_focus_and_yields_to_clean_inspector() {
}
}
}
// Append to app/diagram_focus_tests.rs after the frozen desktop gate.
// Reuses its imports and FocusField, plus Mounted/key from canvas_workspace tests.
async fn modifier_focus_fixture() -> (App, Mounted, Id, Id, Id) {
let mut project = Project::empty();
let a = project
@@ -498,3 +495,99 @@ async fn mounted_window_unfocus_resets_held_shift_before_next_canvas_press() {
assert_eq!(app.document.bundle(), &before.bundle);
assert!(!app.document.can_undo());
}
#[tokio::test]
async fn mounted_selection_strip_keeps_canvas_and_fitted_camera_fixed_for_empty_single_group_and_mixed()
{
use crate::diagram_usage::Target;
let mut project = Project::empty();
let a = project
.add_element(Kind::Activity, Layer::Oa, "First activity", None)
.unwrap();
let b = project
.add_element(
Kind::Activity,
Layer::Oa,
&format!(
"Second activity {}",
"with a very long primary name ".repeat(20)
),
None,
)
.unwrap();
let edge = project.relate(RelationKind::Interaction, "Activity exchange", a, b);
let diagram = project.ensure_diagram(Layer::Oa);
project.diagrams.get_mut(&diagram).unwrap().projection = ProjectionKind::Overview;
let first = project.place(diagram, a, Position { x: 80., y: 80. });
let second = project.place(diagram, b, Position { x: 500., y: 80. });
let mut app = App::with_project(project);
app.page = Page::Diagram;
app.layer = Layer::Oa;
app.active_diagram = Some(diagram);
app.window_size = Size::new(1442., 932.);
let _ = app.update(Message::Select(diagram));
app.fit_to_view = true;
let mut mounted = Mounted::new(&mut app).await;
let baseline = app.document.snapshot();
let bounds = mounted
.geometry(&app)
.bounds(crate::allocation_input::CANVAS_ID);
let camera = (app.zoom, app.pan, app.fit_to_view, app.canvas_size);
assert!(camera.2, "exercise the native resize-triggered refit path");
assert!(app.multiselect_snapshot().unwrap().targets().is_empty());
assert!(bounds.width > 500. && bounds.height > 300.);
for (target, expected) in [
(
Some(Target::Placement(first)),
vec![Target::Placement(first)],
),
(
Some(Target::Placement(second)),
vec![Target::Placement(first), Target::Placement(second)],
),
(
Some(Target::Edge(edge)),
vec![
Target::Placement(first),
Target::Placement(second),
Target::Edge(edge),
],
),
(
Some(Target::Edge(edge)),
vec![Target::Placement(first), Target::Placement(second)],
),
(
Some(Target::Placement(second)),
vec![Target::Placement(first)],
),
(None, vec![]),
] {
let context = app.multiselect_context().unwrap();
let event = match target {
Some(target) => {
multiselect::Event::Choose(context, target, multiselect::Choice::Toggle)
}
None => multiselect::Event::Clear(context),
};
let task = app.update(Message::DiagramSelection(event));
mounted.perform(&mut app, task).await;
mounted.settle(&mut app).await;
assert_eq!(app.multiselect_snapshot().unwrap().targets(), expected);
assert_eq!(
mounted
.geometry(&app)
.bounds(crate::allocation_input::CANVAS_ID),
bounds,
"selection feedback must not change the measured canvas rectangle"
);
assert_eq!(
(app.zoom, app.pan, app.fit_to_view, app.canvas_size),
camera
);
assert_eq!(app.project(), &baseline.decoded.project);
assert_eq!(app.document.bundle(), &baseline.bundle);
assert!(!app.document.can_undo());
assert!(!app.draft_dirty());
}
}
crates/oa-desktop/src/views.rs
+57 -30
View File
@@ -429,32 +429,7 @@ fn diagram(app: &App) -> View<'_, Message> {
chooser = chooser.push(action("View properties", Message::Select(id)));
controls = controls.push(action("Auto-layout", Message::Command(Command::Layout(id))));
}
let mut toolbar = column![chooser, controls].spacing(6);
if let Some(snapshot) = app.multiselect_snapshot().filter(|s| s.targets().len() > 1) {
let mut group_controls = row![label(format!("{} selected", snapshot.targets().len()))]
.spacing(7)
.align_y(iced::Center);
if crate::app::multiselect::placements(&snapshot).is_some() {
group_controls = group_controls.push(action(
"Hide selected",
Message::DiagramSelection(crate::app::multiselect::Event::Hide(snapshot.clone())),
));
}
group_controls = group_controls.push(action(
"Clear selection",
Message::DiagramSelection(crate::app::multiselect::Event::Clear(snapshot.context())),
));
toolbar = toolbar.push(
column![
group_controls,
label(format!(
"Properties edit only {}",
short(app.project().named(app.selected), 36)
))
]
.spacing(3),
);
}
let toolbar = column![chooser, controls].spacing(6);
let projection = app
.active_diagram
.and_then(|id| app.project().diagrams.get(&id))
@@ -634,6 +609,37 @@ fn diagram(app: &App) -> View<'_, Message> {
)
.is_some_and(|(source, target)| source != edge.source || target != edge.target)
});
let selection = app.multiselect_snapshot();
let count = selection.as_ref().map_or(0, |s| s.targets().len());
let mut selection_controls =
row![label(format!("{count} selected")).wrapping(widget::text::Wrapping::None)]
.spacing(7)
.align_y(iced::Center)
.height(32);
if let Some(snapshot) = selection.as_ref().filter(|s| s.targets().len() > 1) {
if crate::app::multiselect::placements(snapshot).is_some() {
selection_controls = selection_controls.push(action(
"Hide selected",
Message::DiagramSelection(crate::app::multiselect::Event::Hide(
snapshot.clone(),
)),
));
}
selection_controls = selection_controls
.push(action(
"Clear selection",
Message::DiagramSelection(crate::app::multiselect::Event::Clear(
snapshot.context(),
)),
))
.push(
label(format!(
"Properties edit only {}",
short(app.project().named(app.selected), 36)
))
.wrapping(widget::text::Wrapping::None),
);
}
let hint = if let Some(feedback) = &app.allocation_drag.feedback {
feedback.text.as_str()
} else if let Some(session) = &app.connection {
@@ -646,16 +652,37 @@ fn diagram(app: &App) -> View<'_, Message> {
} else {
session.tool.hint()
}
} else if count > 1 {
if selection
.as_ref()
.is_some_and(|s| crate::app::multiselect::placements(s).is_some())
&& projection != Some(ProjectionKind::Allocation)
{
"Shift-click to add/remove · drag selected to move · right-click for actions · scroll to zoom"
} else {
"Shift-click to add/remove · right-click for selection actions · Esc clears selection"
}
} else if promoted {
"Expand endpoint containers to edit this connection route"
} else if edge.is_some() {
"Right-click for routing · drag line / bend / label · Insert adds a bend · Esc cancels"
} else {
"Right-click for actions · drag to arrange · scroll to zoom"
"Shift-click to add/remove · right-click for actions · drag to arrange · scroll to zoom"
};
// Status is outside the measured canvas. Clipping prevents long text
// from painting over work headers or the dedicated destination slot.
tools = tools.push(container(label(hint)).height(30).width(Fill).clip(true));
// Selection changes must not resize the measured canvas or refit its
// camera. Reserve both rows even when no group actions are available.
tools = tools.push(
container(
column![
selection_controls,
label(hint).wrapping(widget::text::Wrapping::None)
]
.spacing(3),
)
.height(56)
.width(Fill)
.clip(true),
);
}
column![container(tools).padding(14), canvas].into()
}
harness/multiselect-native-review-20260923.md
+198
View File
@@ -0,0 +1,198 @@
# Independent native evaluator review — 2026-09-23
Read-only review of `crates/oa-core/examples/multiselect_native.rs` at
`12a7352ddef5f69a144ead8219a50916233d63a9`, the local native preparation,
snapshot and capture helpers, and the owning session's proposed UI sequence.
Only this note was edited by the reviewer. No builds, tests, UI actions or
Gitea operations were run. The parent reports core, full desktop and lint gates
passed; no native run was available for acceptance at this review checkpoint.
## Finding and correction
**P2, corrected in the helper during review:** the original
`.local/multiselect-native/prepare-native.py:38–41` recorded the GUI exit code
and binary/fixture immutability booleans, then returned normal helper success
even if the GUI crashed or a recorded input changed. This could mislead a
consumer using process exit status as the gate. The parent added assertions at
lines 42–43 requiring GUI exit zero and all three immutability conditions after
writing the durable receipt. Re-reading the helper confirmed the correction.
No production source or build input changed for this correction, and it was
not runtime-tested by this reviewer.
No additional actionable checker correctness defect was established. A native
acceptance result still requires the actual UI sequence and evidence below;
successful launcher completion alone is insufficient.
## Independence and falsification coverage
- The expected Project starts from the loaded baseline and changes explicitly
enumerated fixture records. Movement expects A, B and the child to move once,
translates only the internal route's pins, and preserves C, one-ended route
pins, attachments, labels, all semantic records, traces and other views.
Hide removes only A/B placements. It preserves the unselected child, stored
routes and model objects. Neither expectation invokes the group planner,
descendant traversal or routing implementation.
- `equal` compares the complete Project, not a selected geometry subset.
Validation, JSON roundtrip and source compilation/roundtrip supplement that
comparison. Completeness warnings in this deliberately partial fixture are
allowed; source discovery errors and compilation failures are rejected.
- The ten checker tests include positive generation/source/command examples
and multiple valid-model corruption falsifiers. The negative cases cover
an omitted or double-moved child, stranded internal pins, moved one-ended
pins, attachment/label drift, semantic and trace drift, other-view and fixed
node changes, invalid movement evidence, destructive Hide and unchanged-mode
semantic drift. Corruptions are validated before comparison and most tests
require the specific whole-Project mismatch, avoiding a fake pass caused
only by malformed input rejection. Planner use is confined to generating
positive/corrupted test candidates, not expected evaluator output.
- Fixture UUIDs are deterministic, generation refuses an existing output
directory, and evidence writes are exclusive. Preparation validates binary
hashes against successful unchanged-input build receipts and requires the
GUI and checker input-manifest hashes to match. The inspected checker
receipt names the reviewed commit; the GUI's then-current started receipt
records the same input hash. Final build/run results remain the parent's gate.
- The snapshot helper copies source into a unique directory, compares the
source file set and hashes before/after copying and after checking, pins the
checker and baseline, and propagates evaluator failure. Source snapshot
evaluation is separate from UI capture, as it should be.
## Acceptance limits for the planned sequence
The parent plans native context Add for A/B, actual drag, saved-model checks,
one-step Undo/Redo with exact source-byte comparison, undo back to baseline,
group Hide with Undo/Redo, and reopen. That is appropriate evidence for the
ordinary saved-view path with these explicit limits:
1. **The movement checker is an invariant evaluator.** It infers the common
delta from saved A and checks bounds, nontrivial length and anchor grid
alignment. A wrong drag direction or magnitude can pass if all required
records share the same admissible delta. The code and manifest already
state this limit. Record native press/release and viewport observations if
claiming intended displacement accuracy; otherwise claim only the verified
common translation and preservation invariants. Existing pointer/gesture
tests provide separate evidence and are not native coordinate evidence.
2. **Model equality does not prove an action happened.** Capture one actual
Undo or Redo and completed Save between the appropriate snapshots. An
unchanged check alone cannot distinguish successful Undo from never making
an edit. Reopen must actually load the saved working source in the GUI and
demonstrate its state; checker roundtrip alone does not verify GUI reopening.
3. **Exact source-byte restoration is a separate comparison.** The snapshot
helper records each source inventory but does not compare different stages.
Explicitly compare all relative paths and hashes: Undo movement to the
original fixture source, Redo movement to the first moved snapshot, and
corresponding baseline/hidden inventories for Hide history. A Project PASS
does not establish identical source formatting or file layout.
4. **Join provenance rather than assuming it.** Tie the captured native window
to the launched PID/executable in `started.json`, identify the opened working
entry, retain the build/input receipts, and require the final fixture and
binary immutability checks. The checker verifies the expected fixture project
ID, not a baked-in baseline hash; baseline authenticity comes from the
generated fixture inventory and unchanged final receipt. Capture JSON and
screenshots alone do not establish which binary produced saved source.
5. **This fixture is OA Activities on an ordinary Overview.** Native context
Add verifies that explicit alternative, not held-Shift injection (unavailable
through the chosen native tool). Mounted focus tests separately exercise
Shift before canvas focus. The native run does not establish an SA Function
workflow, selected ancestor-plus-child deduplication, mixed-port/edge refusal,
busy suspension behavior or every cancellation path. Existing targeted tests
remain separate evidence for those cases; no additional broad native claims
should be inferred.
6. **Scope remains incomplete beyond ordinary saved views.** State, Allocation,
Process/Scenario and aggregate model deletion are not accepted by this
checker or planned native sequence. Their absence is not a new finding in
this bounded review.
These limits qualify what the evidence proves. They do not require replacing
the independently enumerated Project oracle or broadening this native fixture
into the unfinished adapter families.
## Native layout follow-up: selection changes move the canvas
**P2 — group feedback changes the diagram viewport while merely selecting.**
The parent subsequently supplied native run-1 captures. I independently viewed
`.local/multiselect-native/run-1/06-select-a-0.jpg` and
`08-group-selected-0.jpg`; both capture the same native window at 1442 by 932.
The canvas begins near y=314 in the singleton capture and y=370 after forming
the group. A's header moves from approximately y=484 to y=511. The group count,
Hide/Clear actions and primary-properties explanation are readable, but adding
them has moved the diagram under the user's pointer. This is a presentation
finding; it does not assert that selecting changed saved geometry.
The source cause is explicit: `views.rs:434–461` conditionally appends a two-row
group block above the View content control and canvas when the target count
exceeds one. The additional block is about 55 pixels high. The observed node
shift is smaller than the viewport-top shift, consistent with the canvas resize
and current fitted camera reacting to the reduced height. Removing the final
group member or clearing selection reverses the same conditional layout.
The ordinary idle hint (`views.rs:654`) also mentions right-click, drag and zoom
but gives no visible Shift-click discovery cue; neither inspected screenshot
shows such a cue. Context Add is usable, but the user's requested modifier
gesture should be discoverable directly in the ordinary selection UI.
### Smallest space-conscious correction
Move the group feedback into an **always-mounted, fixed-height selection/status
strip at the existing fixed 30-pixel hint location** near `views.rs:658`. Remove
the conditional group block from the toolbar. A roughly 56-pixel strip can
contain:
- A fixed approximately 32-pixel first row with selection count, the existing
group Hide/Clear controls when applicable, and the short explicit disclosure
`Properties edit <primary> only`. Use a bounded/nonwrapping primary label so
long names or a mixed selection cannot increase strip height. Empty and
singleton states keep the same row allocation even with fewer controls.
- A short second line: `Shift-click to add/remove · drag selected to move ·
right-click for actions`. Existing active connection, allocation and route
instructions retain precedence when those modes own input; ordinary idle
selection should expose the modifier hint.
This reuses the existing status location and consumes about 26 more vertical
pixels than today's singleton layout, while recovering about 29 pixels from
today's group layout. The shown central workspace is approximately 860 pixels
wide, sufficient for these compact controls and a bounded primary label without
adding a third row. A fixed outer height, fixed first-row height and nonwrapping
text keep the canvas rectangle identical for zero, one and multiple targets.
Keep this strip outside the canvas; an overlay would obscure diagram content
and complicate hit testing. Simply reserving the current extra 55-pixel toolbar
block would also stop the jump, but spends more permanent vertical space than
reusing the existing status strip.
The narrow verification target is mounted canvas-bounds equality across
0→1→2→1→0 selection transitions, including mixed selection and a long primary
name, with unchanged model/history for those selection changes. A native
before/after capture should confirm stationary node screen positions and readable
controls at the same 1442-pixel window width. No source changes, build or native
interaction were performed by this reviewer for this follow-up.
### Delegated layout correction
The parent subsequently authorized edits to `views.rs`,
`app/diagram_focus_tests.rs` and this note. The conditional toolbar group block
is removed. Every active ordinary diagram now reserves a 56-pixel strip in the
former hint location with a fixed 32-pixel controls row. Count is always shown;
the existing group Hide/Clear intents and primary-only property explanation are
shown for groups. The primary name remains shortened and all strip text is
nonwrapping. Hide still requires whole-set placement admission and all App
event guards are unchanged.
Idle selection hints expose Shift-click add/remove. Supported ordinary placement
groups advertise dragging selected elements together; mixed groups and Allocation
groups receive selection guidance without claiming a movement adapter they do
not have. Active allocation feedback and connection instructions retain priority;
singleton route/promoted-edge hints are preserved. The strip remains outside the
canvas with clipping, and no overlay or global layout policy was introduced.
One mounted regression was added in source:
`mounted_selection_strip_keeps_canvas_and_fitted_camera_fixed_for_empty_single_group_and_mixed`.
It mounts the real App at 1442 by 932 with fitting enabled, a very long primary
name and a visible edge, then checks 0→1→2→3 mixed→2→1→0 selection transitions.
Each step must retain the exact measured canvas rectangle, zoom, pan, fit flag,
observed canvas size, complete Project, source bundle and empty history.
The previous implementation changes the canvas rectangle on the 1→2 transition,
so this verifies the observed regression through production layout and sensors.
Only a static inspection and `git diff --check` were performed for these edits.
Formatting, compilation, regression execution and a fresh native before/after
capture remain owned by the parent. Native run-1 continues against its unchanged
pinned binary and cannot serve as verification of this subsequent source edit.