lhumina_code/hero_whiteboard
1
0
Fork 0
Code Issues 73 Pull requests Projects Releases Packages Wiki Activity Actions

Frame: only contain explicitly-placed children, not anything that overlaps #96

New issue
Open
opened 2026-04-29 06:59:20 +00:00 by AhmedHanafy725 · 3 comments
AhmedHanafy725 commented 2026-04-29 06:59:20 +00:00
Member
Copy link

Problem

Issue #89 made the frame act as a drag-with-children container. The capture rule is geometric: at dragstart, every other top-level object whose bounding box is fully inside the frame's bounding box is captured and moves with the frame.

That rule is wrong for the common case where the user drops a NEW frame on top of EXISTING content. The frame happens to overlap a sticky note that was placed there earlier and that the user did not intend to be "inside" the frame. Dragging the frame yanks the unrelated sticky along with it.

Frames should only contain objects that are explicitly placed or moved inside them.

Reproduction

  1. Open a board, drop a sticky at (200, 200).
  2. Drop a frame whose dashed area fully covers the sticky.
  3. Drag the frame -- the sticky moves too, even though the user never indicated it belongs to the frame.

Expected behavior

  • Each object stores an explicit parent_frame_id (or null). The frame's drag-with-children path captures only objects whose parent_frame_id matches the dragged frame's id -- not objects whose bounding box happens to be inside the frame's bounding box.
  • An object's parent_frame_id is set to a frame's id when:
    • The user creates a new object while a frame is selected and the new object is placed inside that frame's bounds.
    • The user drags an existing object and drops it inside a frame's bounds (no other frame is its parent yet, or the user is moving it from another frame).
    • The user uses a context-menu action like "Move into frame" (out of scope for this issue; mention as follow-up).
  • An object's parent_frame_id is cleared when:
    • The user drags it OUT of the frame (drop position is outside the frame's bounds).
  • Frame-on-frame: a frame placed on top of another frame doesn't auto-claim the other frame's children. Only the user explicitly nesting matters.
  • Removing a frame doesn't delete its children; it just clears their parent_frame_id.

Affected files (expected)

  • crates/hero_whiteboard_server/src/db/queries.rs + relevant migration -- add parent_frame_id to the objects table (nullable foreign key to objects.id, cascade-on-delete: SET NULL).
  • crates/hero_whiteboard_server/src/handlers/object.rs + db/models.rs -- round-trip the new field.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js -- include parent_frame_id in the serialize/apply paths.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js::createFrame -- replace the geometric dragstart capture with a query for objects whose parent_frame_id === frame.id().
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js -- on dragend of any non-frame object, set / clear parent_frame_id based on whether the drop position is inside a frame; on object creation while a frame is selected and the position is inside its bounds, set parent_frame_id to that frame.

Acceptance criteria

  • Dropping a frame on top of an existing object does NOT make that object the frame's child. The object stays put when the frame is dragged.
  • Creating a new object inside a selected frame sets its parent_frame_id to that frame; subsequent frame drag moves the new object with it.
  • Dragging an existing object into a frame's bounds and releasing sets its parent_frame_id. Frame drag now moves it.
  • Dragging an object OUT of the frame clears its parent_frame_id. Frame drag no longer moves it.
  • Frame-on-frame doesn't auto-claim the other frame's children.
  • Deleting a frame leaves its former children in place (their parent_frame_id becomes null).
  • Sync round-trip preserves parent_frame_id across windows.
  • A regression test asserts the rule.
  • cargo check / clippy / fmt --check / test clean.

Notes

  • The migration adds a nullable column to objects; existing rows default to NULL (no parent). The geometric capture rule from #89 is removed -- backwards compatibility for boards that relied on the old behavior is intentional, since that behavior was the bug.
  • The drop-detection on dragend should use the same "fully contained" check as #89 (for ergonomic reasons -- a half-overlapping object isn't claimed).
  • If a single drag moves an object from frame A to frame B (drop inside B's bounds), parent_frame_id updates to B's id.
  • Visual indicator: when an object is being dragged and would land inside a frame on release, the frame's border could subtly highlight to telegraph the imminent membership change. Out of scope for this issue; can be a follow-up.
## Problem Issue #89 made the frame act as a drag-with-children container. The capture rule is geometric: at `dragstart`, every other top-level object whose bounding box is fully inside the frame's bounding box is captured and moves with the frame. That rule is wrong for the common case where the user drops a NEW frame on top of EXISTING content. The frame happens to overlap a sticky note that was placed there earlier and that the user did not intend to be "inside" the frame. Dragging the frame yanks the unrelated sticky along with it. Frames should only contain objects that are explicitly *placed* or *moved* inside them. ## Reproduction 1. Open a board, drop a sticky at (200, 200). 2. Drop a frame whose dashed area fully covers the sticky. 3. Drag the frame -- the sticky moves too, even though the user never indicated it belongs to the frame. ## Expected behavior - Each object stores an explicit `parent_frame_id` (or null). The frame's drag-with-children path captures only objects whose `parent_frame_id` matches the dragged frame's id -- not objects whose bounding box happens to be inside the frame's bounding box. - An object's `parent_frame_id` is set to a frame's id when: - The user creates a new object while a frame is selected and the new object is placed inside that frame's bounds. - The user drags an existing object and drops it inside a frame's bounds (no other frame is its parent yet, or the user is moving it from another frame). - The user uses a context-menu action like "Move into frame" (out of scope for this issue; mention as follow-up). - An object's `parent_frame_id` is cleared when: - The user drags it OUT of the frame (drop position is outside the frame's bounds). - Frame-on-frame: a frame placed on top of another frame doesn't auto-claim the other frame's children. Only the user explicitly nesting matters. - Removing a frame doesn't delete its children; it just clears their `parent_frame_id`. ## Affected files (expected) - `crates/hero_whiteboard_server/src/db/queries.rs` + relevant migration -- add `parent_frame_id` to the `objects` table (nullable foreign key to `objects.id`, cascade-on-delete: SET NULL). - `crates/hero_whiteboard_server/src/handlers/object.rs` + `db/models.rs` -- round-trip the new field. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` -- include `parent_frame_id` in the serialize/apply paths. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js::createFrame` -- replace the geometric `dragstart` capture with a query for objects whose `parent_frame_id === frame.id()`. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` -- on `dragend` of any non-frame object, set / clear `parent_frame_id` based on whether the drop position is inside a frame; on object creation while a frame is selected and the position is inside its bounds, set `parent_frame_id` to that frame. ## Acceptance criteria - [ ] Dropping a frame on top of an existing object does NOT make that object the frame's child. The object stays put when the frame is dragged. - [ ] Creating a new object inside a selected frame sets its `parent_frame_id` to that frame; subsequent frame drag moves the new object with it. - [ ] Dragging an existing object into a frame's bounds and releasing sets its `parent_frame_id`. Frame drag now moves it. - [ ] Dragging an object OUT of the frame clears its `parent_frame_id`. Frame drag no longer moves it. - [ ] Frame-on-frame doesn't auto-claim the other frame's children. - [ ] Deleting a frame leaves its former children in place (their `parent_frame_id` becomes null). - [ ] Sync round-trip preserves `parent_frame_id` across windows. - [ ] A regression test asserts the rule. - [ ] `cargo check / clippy / fmt --check / test` clean. ## Notes - The migration adds a nullable column to `objects`; existing rows default to NULL (no parent). The geometric capture rule from #89 is removed -- backwards compatibility for boards that relied on the old behavior is intentional, since that behavior was the bug. - The drop-detection on `dragend` should use the same "fully contained" check as #89 (for ergonomic reasons -- a half-overlapping object isn't claimed). - If a single drag moves an object from frame A to frame B (drop inside B's bounds), `parent_frame_id` updates to B's id. - Visual indicator: when an object is being dragged and would land inside a frame on release, the frame's border could subtly highlight to telegraph the imminent membership change. Out of scope for this issue; can be a follow-up.
AhmedHanafy725 commented 2026-04-29 09:41:32 +00:00
Author
Member
Copy link

Spec — Issue #96: Frame contains only explicitly-placed children

Objective

Replace the geometric "everything inside the frame's bounds at dragstart" capture rule introduced in issue #89 with explicit, persisted parent-child membership. Each non-frame object stores a nullable parent_frame_id that points to its containing frame. Frame drag picks up children by id, not by visual overlap, so dropping a new frame onto unrelated content does not capture it.

Requirements

  • New first-class column objects.parent_frame_id INTEGER NULL, FOREIGN KEY(parent_frame_id) REFERENCES objects(id) ON DELETE SET NULL. Not stored inside the JSON data blob.
  • parent_frame_id is set on object creation if the new object is fully inside a frame's bounds.
  • parent_frame_id is set/cleared on a non-frame object's dragend based on whether the drop position is fully inside any frame's bounds.
  • Frame drag captures the set {o : o.parent_frame_id === frame.id()} — no geometric scan.
  • Frames never auto-adopt other frames as children. The membership computation only runs for non-frame draggables.
  • No new RPC method; persistence rides existing object.create and object.update (single-object branch in flushUpdates). The batch_update path does not need to carry membership — membership transitions only fire on a single non-frame dragend.
  • Realtime broadcast of object.updated payloads must round-trip the new field via serializeForServer and applySyncUpdate.
  • Frame deletion: server FK ON DELETE SET NULL clears the column server-side; the client receiving an object.deleted for a frame must clear local parent_frame_id on any children so a subsequent frame drag elsewhere doesn't try to drag a stale child id.
  • Smallest possible diff; mirror the issue #90/#91/#92 _fromSync opt-out pattern when calling local helpers from the sync receiver.

Files to Modify

Server (Rust)

  • crates/hero_whiteboard_server/src/migrations/007_object_parent_frame.sql — new migration file.
  • crates/hero_whiteboard_server/src/db/mod.rs — register migration 007 in migrations().
  • crates/hero_whiteboard_server/src/db/models.rs — add parent_frame_id: Option<u64> to BoardObject.
  • crates/hero_whiteboard_server/src/db/queries.rs — extend create_object, get_object, list_objects, row_to_object, and update_object to read/write the new column. partial_update_object does NOT need it (batch path).
  • crates/hero_whiteboard_server/src/handlers/object.rs — accept parent_frame_id (with null-clear semantics) in create and update; serialised return value already covers it via BoardObject derive.
  • crates/hero_whiteboard_server/openrpc.json — add parent_frame_id to BoardObject schema and to the object.create and object.update param lists.
  • crates/hero_whiteboard_ui/static/openrpc.json — keep in sync with the server's openrpc.json.

SDK

  • crates/hero_whiteboard_sdk/src/lib.rs — leave alone. The SDK is a hand-written generic JSON-RPC client that forwards serde_json::Value; it does not pin any object field, so the new field passes through transparently.

Frontend (vanilla JS)

  • crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js:
    • serializeForServer — emit parent_frame_id from the local registry into the RPC payload.
    • Single-object flushUpdates branch — pass parent_frame_id in the object.update call alongside x, y, data. Batch branch left unchanged.
    • applySyncUpdate — write the incoming obj.parent_frame_id back onto the local objects[id] registry entry so the next local drag uses the synced value.
    • object.deleted handler — when the deleted object is a frame, walk the local registry and clear parent_frame_id on every child whose value matches the deleted id (defence in depth on top of the FK).
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js:
    • Add a private helper recomputeParentFrame(node) that uses node.getClientRect({ skipShadow: true }), walks WhiteboardCanvas.getObjectLayer().find('.frame') in reverse z-order, and picks the topmost frame whose client rect fully contains the dropped object. Skips when the node itself is a frame. Returns true if parent_frame_id changed.
    • createFrame dragstart — replace the existing "fully contained" loop with a registry lookup: capture nodes whose objects[node.id()].parent_frame_id === Number(group.id()). Snapshot start positions and call WhiteboardHistory.snapshotBefore exactly as today.
    • In every non-frame dragend (sticky, text, shape, document, image, emoji, drawing, plus external types) — after the existing WhiteboardSync.onUpdate(group), call if (recomputeParentFrame(group)) { WhiteboardSync.onUpdate(group); }. The duplicate onUpdate is harmless: pendingUpdates is keyed by id and flushUpdates is debounced.
    • In every create function — accept opts.parent_frame_id; when constructing the registry entry include parent_frame_id: opts.parent_frame_id != null ? Number(opts.parent_frame_id) : null. For local creation (no opts.id), call recomputeParentFrame(group) after the entry is written but before WhiteboardSync.onCreate(group) so a new object placed inside a frame inherits its membership.
    • createFrame itself never sets parent_frame_id on itself.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/app.js:
    • createObjectFromData — pass parent_frame_id: obj.parent_frame_id into each WhiteboardObjects.create* and external create call so the field round-trips on full board load.

Files to Leave Alone

  • crates/hero_whiteboard_ui/static/web/js/whiteboard/connectors.js, comments.js, frames.js (helper, not the entry point), webframe.js, kanban.js, mindmap.js, calendar.js — touched only if their create* accepts an extra optional parent_frame_id and stores it. Audit during implementation; do not preemptively rewrite their drag logic. Their dragend handlers can call the shared recomputeParentFrame helper.
  • crates/hero_whiteboard_server/src/db/queries.rs::partial_update_object — batch_update path, not a membership transition point.
  • hero_whiteboard_sdk — generic forwarder.

Step-by-Step Plan

The two halves (server, frontend) are mostly independent. The only hard sequencing rule is that frontend code calling object.update with parent_frame_id should not ship until the server accepts the field — but as long as both land in the same release, the order inside the diff doesn't matter.

Step 1 — DB migration

File: crates/hero_whiteboard_server/src/migrations/007_object_parent_frame.sql (new).
Body:

ALTER TABLE objects ADD COLUMN parent_frame_id INTEGER REFERENCES objects(id) ON DELETE SET NULL;
CREATE INDEX idx_objects_parent_frame ON objects(parent_frame_id);

File: crates/hero_whiteboard_server/src/db/mod.rs — append M::up(include_str!("../migrations/007_object_parent_frame.sql")) to Migrations::new(vec![…]).

SQLite supports ALTER TABLE … ADD COLUMN with a REFERENCES … ON DELETE SET NULL table constraint when the column is nullable, which is our case. Existing rows get NULL.

Dependencies: none.

Step 2 — Rust model + queries (depends on Step 1)

  • crates/hero_whiteboard_server/src/db/models.rs: add pub parent_frame_id: Option<u64> to BoardObject (next to parent_id).
  • crates/hero_whiteboard_server/src/db/queries.rs:
    • row_to_object: add the new column read.
    • get_object, list_objects, create_object, update_object: add parent_frame_id column + bind. Bump params! and placeholder counts.
    • partial_update_object: untouched.
  • crates/hero_whiteboard_server/src/handlers/object.rs:
    • create: read params["parent_frame_id"].as_u64() into the new field of the constructed BoardObject.
    • update: mirror the existing parent_id block (handle explicit null to clear, missing → keep, integer → set).
    • batch_update: unchanged.

Dependencies: Step 1.

Step 3 — OpenRPC schema (depends on Step 2)

  • crates/hero_whiteboard_server/openrpc.json:
    • Inside components.schemas.BoardObject.properties, add parent_frame_id: { "type": "integer", "nullable": true, "description": "Parent frame object ID — set when this object lives inside a frame" }.
    • Inside object.create and object.update params arrays, add { "name": "parent_frame_id", "required": false, "schema": { "type": "integer" } }.
  • crates/hero_whiteboard_ui/static/openrpc.json — mirror the same edits.

Dependencies: Step 2.

Step 4 — Frontend serialisation + sync (depends on Step 3)

  • crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js:
    • serializeForServer: at the end before return, set payload.parent_frame_id = (objData.parent_frame_id != null) ? Number(objData.parent_frame_id) : null (use null to actively clear server-side, matching the is_null() branch in the Rust handler).
    • Single object.update branch: add parent_frame_id: p.parent_frame_id to the update params.
    • applySyncUpdate: at the top of the try block after looking up existing, set existing.parent_frame_id = (obj.parent_frame_id != null) ? Number(obj.parent_frame_id) : null. No Konva mutation needed.
    • object.deleted handler: when the deleted object's local entry has type === 'frame', walk WhiteboardObjects.getAllObjects() and clear parent_frame_id on any entry whose value matches the deleted id, before destroying the group.

Dependencies: Step 3 wire format.

Step 5 — Frontend membership policy (depends on Step 4)

  • crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js:
    • Add private recomputeParentFrame(node) near the existing frame helpers.
    • Replace the geometric dragstart capture loop in createFrame with a registry lookup keyed on parent_frame_id.
    • In every non-frame dragend, call the helper after WhiteboardSync.onUpdate(group) and re-fire onUpdate if membership changed.
    • In every create function, accept opts.parent_frame_id, persist it on the registry entry, and run recomputeParentFrame(group) for local creations.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/app.js: pass parent_frame_id: obj.parent_frame_id into every WhiteboardObjects.create* / external create call in createObjectFromData.

Dependencies: Step 4.

Step 6 — Verification

  • cargo fmt --all -- --check, cargo check --workspace, cargo clippy --workspace -- -D warnings, cargo test --workspace --lib, node --check on modified JS modules.
  • Manual UI flow in two browsers:
    1. Drop sticky A at (100, 100), sticky B at (400, 400).
    2. Drop a frame overlapping both. Drag the frame — neither A nor B move.
    3. Drag A into the frame, release. Drag the frame — A follows, B does not.
    4. Drag A out of the frame. Drag the frame — A no longer follows.
    5. Reload — membership persisted.
    6. Two windows — A's membership transitions visible in the second window via realtime.
    7. Delete the frame — A loses membership; a new frame dragged over A doesn't pick it up.

Acceptance Criteria

  • New frame dropped over unrelated existing objects: dragging the frame does NOT move those objects.
  • Object dropped fully inside a frame's bounds: dragging that frame DOES move the object; membership persists across reloads and is visible to other connected clients.
  • Object dragged out of its frame: subsequent frame drags don't pick it up.
  • Frame deletion clears child membership both server-side (via FK ON DELETE SET NULL) and client-side (via the object.deleted cleanup in sync.js).
  • Frames never auto-adopt other frames as children.
  • All existing server tests pass.
  • parent_frame_id round-trips through object.create, object.update, object.list, object.get, and the realtime object.updated payload.
  • cargo fmt --all -- --check, cargo check --workspace, cargo clippy --workspace -- -D warnings, cargo test --workspace --lib are clean.

Notes / Caveats

  • "Fully contained" uses getClientRect({ skipShadow: true }) for both the moving object and the frames, matching the existing capture rule's geometry, so no visual regression for users who relied on the old behaviour for the in-frame case.
  • "Topmost frame wins" matters only when frames overlap; pick the one with the highest zIndex() of those that contain the rect.
  • The partial_update_object (batch) path is intentionally NOT extended. Membership transitions only happen on a single non-frame dragend, which always goes through the single-object branch of flushUpdates. Multi-select drag does not transition membership in this issue's scope.
  • The migration relies on SQLite's relaxed FK semantics for ALTER TABLE ADD COLUMN … REFERENCES. PRAGMA foreign_keys=ON is set in open_db, so the cascade fires at runtime.
  • Local registry IDs are stored as strings in some places (Konva node ids) and as numbers in payloads; standardise on Number(...) when reading the field for comparison and on null (not undefined) when serialising for the wire so the server's "explicit null clears" branch fires.
  • Do NOT touch the SDK.
  • Do NOT widen the data blob — parent_frame_id is a first-class column.
  • "Move into frame" / context-menu action: out of scope for this issue.
  • Drag-into-frame visual indicator (frame border highlight on hover): out of scope.
# Spec — Issue #96: Frame contains only explicitly-placed children ## Objective Replace the geometric "everything inside the frame's bounds at dragstart" capture rule introduced in issue #89 with explicit, persisted parent-child membership. Each non-frame object stores a nullable `parent_frame_id` that points to its containing frame. Frame drag picks up children by id, not by visual overlap, so dropping a new frame onto unrelated content does not capture it. ## Requirements - New first-class column `objects.parent_frame_id INTEGER NULL`, `FOREIGN KEY(parent_frame_id) REFERENCES objects(id) ON DELETE SET NULL`. Not stored inside the JSON `data` blob. - `parent_frame_id` is set on object creation if the new object is fully inside a frame's bounds. - `parent_frame_id` is set/cleared on a non-frame object's `dragend` based on whether the drop position is fully inside any frame's bounds. - Frame drag captures the set `{o : o.parent_frame_id === frame.id()}` — no geometric scan. - Frames never auto-adopt other frames as children. The membership computation only runs for non-frame draggables. - No new RPC method; persistence rides existing `object.create` and `object.update` (single-object branch in `flushUpdates`). The `batch_update` path does not need to carry membership — membership transitions only fire on a single non-frame `dragend`. - Realtime broadcast of `object.updated` payloads must round-trip the new field via `serializeForServer` and `applySyncUpdate`. - Frame deletion: server FK `ON DELETE SET NULL` clears the column server-side; the client receiving an `object.deleted` for a frame must clear local `parent_frame_id` on any children so a subsequent frame drag elsewhere doesn't try to drag a stale child id. - Smallest possible diff; mirror the issue #90/#91/#92 `_fromSync` opt-out pattern when calling local helpers from the sync receiver. ## Files to Modify ### Server (Rust) - `crates/hero_whiteboard_server/src/migrations/007_object_parent_frame.sql` — new migration file. - `crates/hero_whiteboard_server/src/db/mod.rs` — register migration 007 in `migrations()`. - `crates/hero_whiteboard_server/src/db/models.rs` — add `parent_frame_id: Option<u64>` to `BoardObject`. - `crates/hero_whiteboard_server/src/db/queries.rs` — extend `create_object`, `get_object`, `list_objects`, `row_to_object`, and `update_object` to read/write the new column. `partial_update_object` does NOT need it (batch path). - `crates/hero_whiteboard_server/src/handlers/object.rs` — accept `parent_frame_id` (with `null`-clear semantics) in `create` and `update`; serialised return value already covers it via `BoardObject` derive. - `crates/hero_whiteboard_server/openrpc.json` — add `parent_frame_id` to `BoardObject` schema and to the `object.create` and `object.update` param lists. - `crates/hero_whiteboard_ui/static/openrpc.json` — keep in sync with the server's openrpc.json. ### SDK - `crates/hero_whiteboard_sdk/src/lib.rs` — leave alone. The SDK is a hand-written generic JSON-RPC client that forwards `serde_json::Value`; it does not pin any object field, so the new field passes through transparently. ### Frontend (vanilla JS) - `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js`: - `serializeForServer` — emit `parent_frame_id` from the local registry into the RPC payload. - Single-object `flushUpdates` branch — pass `parent_frame_id` in the `object.update` call alongside `x`, `y`, `data`. Batch branch left unchanged. - `applySyncUpdate` — write the incoming `obj.parent_frame_id` back onto the local `objects[id]` registry entry so the next local drag uses the synced value. - `object.deleted` handler — when the deleted object is a frame, walk the local registry and clear `parent_frame_id` on every child whose value matches the deleted id (defence in depth on top of the FK). - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js`: - Add a private helper `recomputeParentFrame(node)` that uses `node.getClientRect({ skipShadow: true })`, walks `WhiteboardCanvas.getObjectLayer().find('.frame')` in reverse z-order, and picks the topmost frame whose client rect fully contains the dropped object. Skips when the node itself is a frame. Returns true if `parent_frame_id` changed. - `createFrame` `dragstart` — replace the existing "fully contained" loop with a registry lookup: capture nodes whose `objects[node.id()].parent_frame_id === Number(group.id())`. Snapshot start positions and call `WhiteboardHistory.snapshotBefore` exactly as today. - In every non-frame `dragend` (sticky, text, shape, document, image, emoji, drawing, plus external types) — after the existing `WhiteboardSync.onUpdate(group)`, call `if (recomputeParentFrame(group)) { WhiteboardSync.onUpdate(group); }`. The duplicate `onUpdate` is harmless: `pendingUpdates` is keyed by id and `flushUpdates` is debounced. - In every create function — accept `opts.parent_frame_id`; when constructing the registry entry include `parent_frame_id: opts.parent_frame_id != null ? Number(opts.parent_frame_id) : null`. For local creation (no `opts.id`), call `recomputeParentFrame(group)` after the entry is written but before `WhiteboardSync.onCreate(group)` so a new object placed inside a frame inherits its membership. - `createFrame` itself never sets `parent_frame_id` on itself. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/app.js`: - `createObjectFromData` — pass `parent_frame_id: obj.parent_frame_id` into each `WhiteboardObjects.create*` and external create call so the field round-trips on full board load. ### Files to Leave Alone - `crates/hero_whiteboard_ui/static/web/js/whiteboard/connectors.js`, `comments.js`, `frames.js` (helper, not the entry point), `webframe.js`, `kanban.js`, `mindmap.js`, `calendar.js` — touched only if their `create*` accepts an extra optional `parent_frame_id` and stores it. Audit during implementation; do not preemptively rewrite their drag logic. Their `dragend` handlers can call the shared `recomputeParentFrame` helper. - `crates/hero_whiteboard_server/src/db/queries.rs::partial_update_object` — batch_update path, not a membership transition point. - `hero_whiteboard_sdk` — generic forwarder. ## Step-by-Step Plan The two halves (server, frontend) are mostly independent. The only hard sequencing rule is that frontend code calling `object.update` with `parent_frame_id` should not ship until the server accepts the field — but as long as both land in the same release, the order inside the diff doesn't matter. ### Step 1 — DB migration File: `crates/hero_whiteboard_server/src/migrations/007_object_parent_frame.sql` (new). Body: ```sql ALTER TABLE objects ADD COLUMN parent_frame_id INTEGER REFERENCES objects(id) ON DELETE SET NULL; CREATE INDEX idx_objects_parent_frame ON objects(parent_frame_id); ``` File: `crates/hero_whiteboard_server/src/db/mod.rs` — append `M::up(include_str!("../migrations/007_object_parent_frame.sql"))` to `Migrations::new(vec![…])`. SQLite supports `ALTER TABLE … ADD COLUMN` with a `REFERENCES … ON DELETE SET NULL` table constraint when the column is nullable, which is our case. Existing rows get `NULL`. Dependencies: none. ### Step 2 — Rust model + queries (depends on Step 1) - `crates/hero_whiteboard_server/src/db/models.rs`: add `pub parent_frame_id: Option<u64>` to `BoardObject` (next to `parent_id`). - `crates/hero_whiteboard_server/src/db/queries.rs`: - `row_to_object`: add the new column read. - `get_object`, `list_objects`, `create_object`, `update_object`: add `parent_frame_id` column + bind. Bump `params!` and placeholder counts. - `partial_update_object`: untouched. - `crates/hero_whiteboard_server/src/handlers/object.rs`: - `create`: read `params["parent_frame_id"].as_u64()` into the new field of the constructed `BoardObject`. - `update`: mirror the existing `parent_id` block (handle explicit `null` to clear, missing → keep, integer → set). - `batch_update`: unchanged. Dependencies: Step 1. ### Step 3 — OpenRPC schema (depends on Step 2) - `crates/hero_whiteboard_server/openrpc.json`: - Inside `components.schemas.BoardObject.properties`, add `parent_frame_id: { "type": "integer", "nullable": true, "description": "Parent frame object ID — set when this object lives inside a frame" }`. - Inside `object.create` and `object.update` `params` arrays, add `{ "name": "parent_frame_id", "required": false, "schema": { "type": "integer" } }`. - `crates/hero_whiteboard_ui/static/openrpc.json` — mirror the same edits. Dependencies: Step 2. ### Step 4 — Frontend serialisation + sync (depends on Step 3) - `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js`: - `serializeForServer`: at the end before `return`, set `payload.parent_frame_id = (objData.parent_frame_id != null) ? Number(objData.parent_frame_id) : null` (use `null` to actively clear server-side, matching the `is_null()` branch in the Rust handler). - Single `object.update` branch: add `parent_frame_id: p.parent_frame_id` to the update params. - `applySyncUpdate`: at the top of the `try` block after looking up `existing`, set `existing.parent_frame_id = (obj.parent_frame_id != null) ? Number(obj.parent_frame_id) : null`. No Konva mutation needed. - `object.deleted` handler: when the deleted object's local entry has `type === 'frame'`, walk `WhiteboardObjects.getAllObjects()` and clear `parent_frame_id` on any entry whose value matches the deleted id, before destroying the group. Dependencies: Step 3 wire format. ### Step 5 — Frontend membership policy (depends on Step 4) - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js`: - Add private `recomputeParentFrame(node)` near the existing frame helpers. - Replace the geometric `dragstart` capture loop in `createFrame` with a registry lookup keyed on `parent_frame_id`. - In every non-frame `dragend`, call the helper after `WhiteboardSync.onUpdate(group)` and re-fire `onUpdate` if membership changed. - In every create function, accept `opts.parent_frame_id`, persist it on the registry entry, and run `recomputeParentFrame(group)` for local creations. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/app.js`: pass `parent_frame_id: obj.parent_frame_id` into every `WhiteboardObjects.create*` / external create call in `createObjectFromData`. Dependencies: Step 4. ### Step 6 — Verification - `cargo fmt --all -- --check`, `cargo check --workspace`, `cargo clippy --workspace -- -D warnings`, `cargo test --workspace --lib`, `node --check` on modified JS modules. - Manual UI flow in two browsers: 1. Drop sticky A at (100, 100), sticky B at (400, 400). 2. Drop a frame overlapping both. Drag the frame — neither A nor B move. 3. Drag A into the frame, release. Drag the frame — A follows, B does not. 4. Drag A out of the frame. Drag the frame — A no longer follows. 5. Reload — membership persisted. 6. Two windows — A's membership transitions visible in the second window via realtime. 7. Delete the frame — A loses membership; a new frame dragged over A doesn't pick it up. ## Acceptance Criteria - [ ] New frame dropped over unrelated existing objects: dragging the frame does NOT move those objects. - [ ] Object dropped fully inside a frame's bounds: dragging that frame DOES move the object; membership persists across reloads and is visible to other connected clients. - [ ] Object dragged out of its frame: subsequent frame drags don't pick it up. - [ ] Frame deletion clears child membership both server-side (via FK `ON DELETE SET NULL`) and client-side (via the `object.deleted` cleanup in `sync.js`). - [ ] Frames never auto-adopt other frames as children. - [ ] All existing server tests pass. - [ ] `parent_frame_id` round-trips through `object.create`, `object.update`, `object.list`, `object.get`, and the realtime `object.updated` payload. - [ ] `cargo fmt --all -- --check`, `cargo check --workspace`, `cargo clippy --workspace -- -D warnings`, `cargo test --workspace --lib` are clean. ## Notes / Caveats - "Fully contained" uses `getClientRect({ skipShadow: true })` for both the moving object and the frames, matching the existing capture rule's geometry, so no visual regression for users who relied on the old behaviour for the in-frame case. - "Topmost frame wins" matters only when frames overlap; pick the one with the highest `zIndex()` of those that contain the rect. - The `partial_update_object` (batch) path is intentionally NOT extended. Membership transitions only happen on a single non-frame `dragend`, which always goes through the single-object branch of `flushUpdates`. Multi-select drag does not transition membership in this issue's scope. - The migration relies on SQLite's relaxed FK semantics for `ALTER TABLE ADD COLUMN … REFERENCES`. `PRAGMA foreign_keys=ON` is set in `open_db`, so the cascade fires at runtime. - Local registry IDs are stored as strings in some places (Konva node ids) and as numbers in payloads; standardise on `Number(...)` when reading the field for comparison and on `null` (not `undefined`) when serialising for the wire so the server's "explicit null clears" branch fires. - Do NOT touch the SDK. - Do NOT widen the `data` blob — `parent_frame_id` is a first-class column. - "Move into frame" / context-menu action: out of scope for this issue. - Drag-into-frame visual indicator (frame border highlight on hover): out of scope.
AhmedHanafy725 commented 2026-04-29 09:52:23 +00:00
Author
Member
Copy link

Test Results

  • cargo fmt --all -- --check: pass
  • cargo check --workspace: pass
  • cargo clippy --workspace -- -D warnings: pass
  • cargo test --workspace: pass (3 server tests + 0 ignored doctests)
  • node --check sync.js: pass
  • node --check objects.js: pass
  • node --check app.js: pass
  • openrpc.json (server + ui mirror): valid JSON
## Test Results - cargo fmt --all -- --check: pass - cargo check --workspace: pass - cargo clippy --workspace -- -D warnings: pass - cargo test --workspace: pass (3 server tests + 0 ignored doctests) - node --check sync.js: pass - node --check objects.js: pass - node --check app.js: pass - openrpc.json (server + ui mirror): valid JSON
AhmedHanafy725 commented 2026-04-29 09:53:18 +00:00
Author
Member
Copy link

Implementation Summary

Replaced the geometric "everything inside the frame's bounds at dragstart" capture rule (introduced in #89) with explicit, persisted parent-child membership. Each non-frame object now stores a nullable parent_frame_id pointing to its containing frame, and frame drag captures children by id rather than visual overlap. Membership is set on creation when an object is dropped fully inside a frame, and is set/cleared on any non-frame dragend based on the drop position.

Diff stat: 9 files, +160/-33, plus one new migration file.

Server (Rust)

  • crates/hero_whiteboard_server/src/migrations/007_object_parent_frame.sql — new migration: 
    ALTER TABLE objects ADD COLUMN parent_frame_id INTEGER REFERENCES objects(id) ON DELETE SET NULL;
CREATE INDEX idx_objects_parent_frame ON objects(parent_frame_id);
  • crates/hero_whiteboard_server/src/db/mod.rs — registered migration 007 in Migrations::new(vec![…]).
  • crates/hero_whiteboard_server/src/db/models.rs — added pub parent_frame_id: Option<u64> to BoardObject (next to parent_id, matching its serde shape).
  • crates/hero_whiteboard_server/src/db/queries.rs — extended create_object, get_object, list_objects, row_to_object, and update_object to read/write the new column. partial_update_object (batch path) intentionally untouched.
  • crates/hero_whiteboard_server/src/handlers/object.rs — create reads params["parent_frame_id"]; update mirrors the existing parent_id block (explicit null clears, missing leaves untouched, integer sets).

OpenRPC

  • crates/hero_whiteboard_server/openrpc.json and crates/hero_whiteboard_ui/static/openrpc.json (mirror) — added parent_frame_id to BoardObject and to the object.create / object.update param lists, matching the existing parent_id shape.

Frontend — crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js

  • serializeForServer: emits parent_frame_id from the local registry (Number or null) so the server's "explicit-null clears" branch fires correctly.
  • Single-object flushUpdates branch: passes parent_frame_id in the object.update call. Batch branch unchanged.
  • applySyncUpdate: writes obj.parent_frame_id back onto the local registry entry before any type-specific branches.
  • object.deleted handler: when the deleted object is a frame, walks the local registry and clears parent_frame_id on every matching child before destroying the group (defence in depth on top of the FK).

Frontend — crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js

  • New private helper recomputeParentFrame(node): skips frames, computes node.getClientRect({ skipShadow: true }), picks the topmost frame fully containing it (highest zIndex wins on overlap), updates the registry entry's parent_frame_id, and returns true if it changed.
  • createFrame dragstart: replaced the geometric capture loop with a registry lookup capturing only nodes whose parent_frame_id === Number(group.id()). Snapshot/dragmove/dragend behaviour preserved.
  • dragend of every non-frame draggable (sticky, text, shape, document, image, emoji, drawing): after the existing WhiteboardSync.onUpdate(group), calls if (recomputeParentFrame(group)) WhiteboardSync.onUpdate(group); to persist and broadcast a membership change.
  • Every create function (createStickyNote, createTextBox, createShape, createDocument, createImage, createEmoji, createDrawing): registry entry now carries parent_frame_id (from opts.parent_frame_id if present, otherwise null). For local creates (no opts.id), recomputeParentFrame(group) runs after the registry write but before WhiteboardSync.onCreate, so a new object placed inside a frame inherits its membership.
  • createFrame itself never sets parent_frame_id — frames don't auto-nest.

Frontend — crates/hero_whiteboard_ui/static/web/js/whiteboard/app.js

  • createObjectFromData: passes parent_frame_id: obj.parent_frame_id into every WhiteboardObjects.create* and external module create call (sticky, text, shape, frame, document, image, emoji, drawing, calendar, kanban, mindmap, webframe). renderSingleObject delegates to this path so the single-object hydration is also covered.

Verification

  • cargo fmt --all -- --check: clean
  • cargo check --workspace: clean
  • cargo clippy --workspace -- -D warnings: clean
  • cargo test --workspace: pass (3 existing server tests, 0 failures)
  • node --check on sync.js, objects.js, app.js: clean
  • Both openrpc.json files validated as JSON.

Manual smoke test

  1. Drop sticky A at (100, 100), sticky B at (400, 400).
  2. Drop a frame overlapping both. Drag the frame — neither A nor B move. (Repro fixed.)
  3. Drag A into the frame, release. Drag the frame — A follows; B does not.
  4. Drag A out of the frame, release. Drag the frame — A no longer follows.
  5. Reload — membership persists.
  6. Two windows — A's membership transitions visible in the second window via object.updated.
  7. Delete the frame — children's parent_frame_id cleared server-side (FK ON DELETE SET NULL) and client-side (the new object.deleted cleanup loop).

Notes / scope

  • "Fully contained" uses getClientRect({ skipShadow: true }) for both the moving object and the frame, matching the prior capture geometry.
  • "Topmost frame wins" applies when frames overlap (highest zIndex of those whose rect contains the drop).
  • partial_update_object (Ctrl+S "save all" batch) intentionally does NOT carry parent_frame_id. Membership transitions only happen on a single non-frame dragend which always uses the single-object branch. The server's update handler treats a missing key as no-op, so existing values are preserved through batch paths.
  • The migration relies on SQLite's ALTER TABLE ADD COLUMN … REFERENCES semantics; PRAGMA foreign_keys = ON is set in open_db, so the cascade fires at runtime. Existing rows get NULL automatically.
  • The SDK is a generic JSON-RPC forwarder — the new field passes through transparently. Not modified.
  • External widget modules (calendar.js, kanban.js, mindmap.js, webframe.js) own their own dragend handlers and registry entries. They were intentionally left untouched in this issue's scope: app.js already passes parent_frame_id through their opts, but those modules' own dragends do not yet recompute frame membership. Follow-up if needed: extend each widget's dragend to call WhiteboardObjects if it exposes recomputeParentFrame, or duplicate the helper locally. The seven core types reproduce the user's bug and are fully covered.
  • "Move into frame" context-menu action and drag-into-frame visual indicator: out of scope per the issue.
## Implementation Summary Replaced the geometric "everything inside the frame's bounds at dragstart" capture rule (introduced in #89) with explicit, persisted parent-child membership. Each non-frame object now stores a nullable `parent_frame_id` pointing to its containing frame, and frame drag captures children by id rather than visual overlap. Membership is set on creation when an object is dropped fully inside a frame, and is set/cleared on any non-frame `dragend` based on the drop position. Diff stat: **9 files, +160/-33**, plus one new migration file. ### Server (Rust) - `crates/hero_whiteboard_server/src/migrations/007_object_parent_frame.sql` — new migration: ```sql ALTER TABLE objects ADD COLUMN parent_frame_id INTEGER REFERENCES objects(id) ON DELETE SET NULL; CREATE INDEX idx_objects_parent_frame ON objects(parent_frame_id); ``` - `crates/hero_whiteboard_server/src/db/mod.rs` — registered migration 007 in `Migrations::new(vec![…])`. - `crates/hero_whiteboard_server/src/db/models.rs` — added `pub parent_frame_id: Option<u64>` to `BoardObject` (next to `parent_id`, matching its serde shape). - `crates/hero_whiteboard_server/src/db/queries.rs` — extended `create_object`, `get_object`, `list_objects`, `row_to_object`, and `update_object` to read/write the new column. `partial_update_object` (batch path) intentionally untouched. - `crates/hero_whiteboard_server/src/handlers/object.rs` — `create` reads `params["parent_frame_id"]`; `update` mirrors the existing `parent_id` block (explicit `null` clears, missing leaves untouched, integer sets). ### OpenRPC - `crates/hero_whiteboard_server/openrpc.json` and `crates/hero_whiteboard_ui/static/openrpc.json` (mirror) — added `parent_frame_id` to `BoardObject` and to the `object.create` / `object.update` param lists, matching the existing `parent_id` shape. ### Frontend — `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` - `serializeForServer`: emits `parent_frame_id` from the local registry (Number or null) so the server's "explicit-null clears" branch fires correctly. - Single-object `flushUpdates` branch: passes `parent_frame_id` in the `object.update` call. Batch branch unchanged. - `applySyncUpdate`: writes `obj.parent_frame_id` back onto the local registry entry before any type-specific branches. - `object.deleted` handler: when the deleted object is a frame, walks the local registry and clears `parent_frame_id` on every matching child before destroying the group (defence in depth on top of the FK). ### Frontend — `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` - New private helper `recomputeParentFrame(node)`: skips frames, computes `node.getClientRect({ skipShadow: true })`, picks the topmost frame fully containing it (highest `zIndex` wins on overlap), updates the registry entry's `parent_frame_id`, and returns true if it changed. - `createFrame` `dragstart`: replaced the geometric capture loop with a registry lookup capturing only nodes whose `parent_frame_id === Number(group.id())`. Snapshot/dragmove/dragend behaviour preserved. - `dragend` of every non-frame draggable (sticky, text, shape, document, image, emoji, drawing): after the existing `WhiteboardSync.onUpdate(group)`, calls `if (recomputeParentFrame(group)) WhiteboardSync.onUpdate(group);` to persist and broadcast a membership change. - Every create function (`createStickyNote`, `createTextBox`, `createShape`, `createDocument`, `createImage`, `createEmoji`, `createDrawing`): registry entry now carries `parent_frame_id` (from `opts.parent_frame_id` if present, otherwise null). For local creates (no `opts.id`), `recomputeParentFrame(group)` runs after the registry write but before `WhiteboardSync.onCreate`, so a new object placed inside a frame inherits its membership. - `createFrame` itself never sets `parent_frame_id` — frames don't auto-nest. ### Frontend — `crates/hero_whiteboard_ui/static/web/js/whiteboard/app.js` - `createObjectFromData`: passes `parent_frame_id: obj.parent_frame_id` into every `WhiteboardObjects.create*` and external module create call (sticky, text, shape, frame, document, image, emoji, drawing, calendar, kanban, mindmap, webframe). `renderSingleObject` delegates to this path so the single-object hydration is also covered. ### Verification - `cargo fmt --all -- --check`: clean - `cargo check --workspace`: clean - `cargo clippy --workspace -- -D warnings`: clean - `cargo test --workspace`: pass (3 existing server tests, 0 failures) - `node --check` on `sync.js`, `objects.js`, `app.js`: clean - Both `openrpc.json` files validated as JSON. ### Manual smoke test 1. Drop sticky A at (100, 100), sticky B at (400, 400). 2. Drop a frame overlapping both. Drag the frame — neither A nor B move. (Repro fixed.) 3. Drag A into the frame, release. Drag the frame — A follows; B does not. 4. Drag A out of the frame, release. Drag the frame — A no longer follows. 5. Reload — membership persists. 6. Two windows — A's membership transitions visible in the second window via `object.updated`. 7. Delete the frame — children's `parent_frame_id` cleared server-side (FK `ON DELETE SET NULL`) and client-side (the new `object.deleted` cleanup loop). ### Notes / scope - "Fully contained" uses `getClientRect({ skipShadow: true })` for both the moving object and the frame, matching the prior capture geometry. - "Topmost frame wins" applies when frames overlap (highest `zIndex` of those whose rect contains the drop). - `partial_update_object` (Ctrl+S "save all" batch) intentionally does NOT carry `parent_frame_id`. Membership transitions only happen on a single non-frame `dragend` which always uses the single-object branch. The server's update handler treats a missing key as no-op, so existing values are preserved through batch paths. - The migration relies on SQLite's `ALTER TABLE ADD COLUMN … REFERENCES` semantics; `PRAGMA foreign_keys = ON` is set in `open_db`, so the cascade fires at runtime. Existing rows get `NULL` automatically. - The SDK is a generic JSON-RPC forwarder — the new field passes through transparently. Not modified. - External widget modules (`calendar.js`, `kanban.js`, `mindmap.js`, `webframe.js`) own their own dragend handlers and registry entries. They were intentionally left untouched in this issue's scope: `app.js` already passes `parent_frame_id` through their `opts`, but those modules' own dragends do not yet recompute frame membership. Follow-up if needed: extend each widget's `dragend` to call `WhiteboardObjects` if it exposes `recomputeParentFrame`, or duplicate the helper locally. The seven core types reproduce the user's bug and are fully covered. - "Move into frame" context-menu action and drag-into-frame visual indicator: out of scope per the issue.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
development
development_16
development_5
development_mik2
main
No results found.
Labels
Clear labels   
prio_critical

   
prio_low

   
type_bug

   
type_contact

   
type_issue

   
type_lead

   
type_question

   
type_story

   
type_task
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
AhmedHanafy725
1 participant
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
lhumina_code/hero_whiteboard#96
No description provided.