lhumina_code/hero_whiteboard

Fork 0

Whiteboard: draw / eraser / rubber-band can get stuck "on" if mouseup happens outside the Konva canvas #185

New issue

Open

opened 2026-05-13 12:20:52 +00:00 by AhmedHanafy725 · 2 comments

AhmedHanafy725 commented

2026-05-13 12:20:52 +00:00

Member

Problem (critical)

When the user is in Draw mode and releases the mouse button while the cursor is over the floating subtoolbar (or any DOM element outside the Konva stage container, like the top header, the property panel, etc.), the stroke never ends:

User mousedowns on canvas → Konva fires mousedown → drawLine Konva preview line is created (tools.js:539-549).
User drags the cursor off the canvas onto the subtoolbar while still holding.
User releases the button. mouseup fires on the subtoolbar DOM element, not on Konva (they are sibling DOM nodes, not nested). Konvas stage mouseup handler (tools.js:729) never runs.
drawLine is stuck non-null. The orphan preview line is still on the object layer.
The next time the cursor moves over the canvas, stage.on("mousemove") fires unconditionally on hover — the currentTool === "draw" && drawLine branch (tools.js:658-662) keeps appending points to the orphan line. The user appears to be drawing without holding the button.
The stroke is never persisted (createDrawing is only called in onMouseUp at line 737), so a page refresh wipes it.

Same class of bug also affects:

Eraser — isErasing flag (tools.js:551, 748-754) stays true; subsequent cursor moves keep erasing.
Rubber-band selection — isSelecting flag (tools.js:480-482, 677-680); the rubber-band rectangle can be left visible and tracking the cursor.

Root cause

Konvas mousedown/mousemove/mouseup are attached to the stage container <div id="whiteboard-container">. The subtoolbar (<div id="subtoolbar">), main toolbar, and other UI overlays are siblings of that container. A button release over any of them does not propagate to Konva. Mid-stroke state (drawLine, isErasing, isSelecting) is never cleaned up.

Expected

Releasing the mouse button outside the canvas while a stroke / eraser drag / rubber-band is in progress ends the gesture cleanly: persist the drawing (if it has points), reset state, hide the rubber-band rectangle, commit the eraser drag.

Acceptance

Draw a stroke that ends with the cursor released outside the canvas → the stroke is finalised as a persistent drawing (visible after refresh) and drawLine is reset.
Subsequent cursor movement over the canvas does NOT continue drawing.
Eraser drag released outside the canvas → eraser state resets, partial cuts committed.
Rubber-band drag released outside the canvas → selection is finalised against the current rectangle and the rectangle is hidden.
No regression to normal in-canvas mouseup behaviour.
No double-commit if mouseup happens inside the canvas (the in-canvas handler should remain authoritative).

Approach (sketch)

Add a document (or window) level mouseup safety net in tools.js:setup() that calls the same end-gesture cleanup whenever drawLine / isErasing / isSelecting is non-null. Use a guard flag so the Konva mouseup handler doesnt double-fire if both run for the same release.

Out of scope

Mouse leaving the window entirely (mouseleave on document) — pointer capture handles that on most browsers; if it becomes an issue, file separately.

## Problem (critical) When the user is in **Draw** mode and releases the mouse button while the cursor is over the floating **subtoolbar** (or any DOM element outside the Konva stage container, like the top header, the property panel, etc.), the stroke never ends: 1. User mousedowns on canvas → Konva fires `mousedown` → `drawLine` Konva preview line is created (`tools.js:539-549`). 2. User drags the cursor off the canvas onto the subtoolbar while still holding. 3. User releases the button. **`mouseup` fires on the subtoolbar DOM element, not on Konva** (they are sibling DOM nodes, not nested). Konvas stage `mouseup` handler (`tools.js:729`) never runs. 4. `drawLine` is stuck non-null. The orphan preview line is still on the object layer. 5. The next time the cursor moves over the canvas, `stage.on("mousemove")` fires unconditionally on hover — the `currentTool === "draw" && drawLine` branch (`tools.js:658-662`) keeps appending points to the orphan line. **The user appears to be drawing without holding the button.** 6. The stroke is never persisted (`createDrawing` is only called in `onMouseUp` at line 737), so a page refresh wipes it. Same class of bug also affects: - **Eraser** — `isErasing` flag (`tools.js:551, 748-754`) stays true; subsequent cursor moves keep erasing. - **Rubber-band selection** — `isSelecting` flag (`tools.js:480-482, 677-680`); the rubber-band rectangle can be left visible and tracking the cursor. ## Root cause Konvas `mousedown`/`mousemove`/`mouseup` are attached to the stage container `<div id="whiteboard-container">`. The subtoolbar (`<div id="subtoolbar">`), main toolbar, and other UI overlays are siblings of that container. A button release over any of them does not propagate to Konva. Mid-stroke state (`drawLine`, `isErasing`, `isSelecting`) is never cleaned up. ## Expected Releasing the mouse button outside the canvas while a stroke / eraser drag / rubber-band is in progress ends the gesture cleanly: persist the drawing (if it has points), reset state, hide the rubber-band rectangle, commit the eraser drag. ## Acceptance - [ ] Draw a stroke that ends with the cursor released outside the canvas → the stroke is finalised as a persistent drawing (visible after refresh) and `drawLine` is reset. - [ ] Subsequent cursor movement over the canvas does NOT continue drawing. - [ ] Eraser drag released outside the canvas → eraser state resets, partial cuts committed. - [ ] Rubber-band drag released outside the canvas → selection is finalised against the current rectangle and the rectangle is hidden. - [ ] No regression to normal in-canvas mouseup behaviour. - [ ] No double-commit if mouseup happens inside the canvas (the in-canvas handler should remain authoritative). ## Approach (sketch) Add a `document` (or `window`) level `mouseup` safety net in `tools.js:setup()` that calls the same end-gesture cleanup whenever `drawLine` / `isErasing` / `isSelecting` is non-null. Use a guard flag so the Konva `mouseup` handler doesnt double-fire if both run for the same release. ## Out of scope Mouse leaving the window entirely (`mouseleave` on document) — pointer capture handles that on most browsers; if it becomes an issue, file separately.

AhmedHanafy725 commented

2026-05-13 12:21:33 +00:00

Author

Member

Implementation Spec for Issue #185

Objective

Releasing the mouse button outside the Konva stage during an in-progress draw / eraser / rubber-band gesture must end the gesture cleanly: persist the drawing, commit the eraser drag, finalise the rubber-band selection. Subsequent cursor movement must not keep drawing / erasing / extending the selection rectangle.

Approach

Add a single document-level mouseup safety net in tools.js:setup(). It calls the existing Konva onMouseUp(e) handler when any in-progress gesture flag is non-null (drawLine, isErasing, isSelecting). The Konva-level handler still runs first when the release happens inside the canvas; the document-level handler is a fallback when it doesn't.

Use a one-shot guard so onMouseUp doesn't double-fire for a single release:

Set _gestureEndedThisCycle = true inside onMouseUp after it runs.
The document fallback short-circuits if the flag is already set, then clears it on the next animation frame so subsequent gestures aren't blocked.

File to modify

crates/hero_whiteboard_admin/static/web/js/whiteboard/tools.js

Implementation Plan

Step 1: Refactor `onMouseUp` into a reusable end-gesture function

Add a guard flag and a thin wrapper that both the Konva mouseup and the new document fallback call. Concretely:

var _gestureEndedThisCycle = false;

function endActiveGesture(e) {
    if (_gestureEndedThisCycle) return;
    _gestureEndedThisCycle = true;
    // Clear the guard at the next tick so the next gesture can run.
    setTimeout(function () { _gestureEndedThisCycle = false; }, 0);
    onMouseUp(e);
}

Replace the existing Konva binding:

stage.on('mouseup', onMouseUp);

with:

stage.on('mouseup', endActiveGesture);

Step 2: Add the document-level safety net in `setup()`

After the existing stage event bindings (around tools.js:378), add:

// If the cursor releases outside the Konva stage (e.g. over the
// subtoolbar) and a gesture is in progress, end it the same way an
// in-canvas mouseup would. Without this, drawLine / isErasing /
// isSelecting can stay non-null and the next mousemove keeps
// drawing / erasing / extending the rubber-band.
document.addEventListener('mouseup', function (e) {
    if (drawLine || isErasing || isSelecting) {
        endActiveGesture(e);
    }
});

onMouseUp already handles the three branches correctly, so we don't need to duplicate the cleanup logic.

Step 3: Build a fake Konva-shaped event for the rubber-band branch

onMouseUp reads e.evt for things like lastClientX/Y — but only for the path that fires while mid-drag (within onMouseMove). The rubber-band end branch uses selectionRect.getClientRect(), not the event coords directly. The other branches (draw / eraser) don't read from the event at all. So passing the raw DOM MouseEvent as e is safe — onMouseUp doesn't access e.evt from mouseup itself.

If a regression is observed, wrap e as { evt: e } before calling onMouseUp(e) to mimic the Konva shape — but a quick grep of onMouseUp shows no e.evt access in the mouseup-side code paths.

Acceptance Criteria

Draw a stroke that ends with the cursor over the subtoolbar → the stroke is persisted as a drawing object and the in-progress preview is cleaned up. Hovering the canvas afterwards does NOT draw.
Same test with cursor released outside the entire viewport (document mouseup still fires for the latter case in most browsers) → same outcome.
Eraser drag released outside the canvas → eraser state resets; in precision mode the partial cut is committed.
Rubber-band drag released outside the canvas → selection finalises against the current rectangle and the rectangle is hidden.
No double-commit when mouseup happens INSIDE the canvas (the _gestureEndedThisCycle guard prevents the document fallback from running after the stage handler already did).
No regression to in-canvas behaviour for any tool.

Notes

The document listener is added once, in setup(). No teardown needed (the page lifetime owns it).
This is a minimal change — no new module structure, no new state machine. Just guard + fallback.
Pointer-events / mouse-leaves-window cases that don't fire mouseup on document (some browsers / OS combinations) are out of scope; if they become an issue, add a pointercancel / blur fallback.

## Implementation Spec for Issue #185 ### Objective Releasing the mouse button outside the Konva stage during an in-progress draw / eraser / rubber-band gesture must end the gesture cleanly: persist the drawing, commit the eraser drag, finalise the rubber-band selection. Subsequent cursor movement must not keep drawing / erasing / extending the selection rectangle. ### Approach Add a single `document`-level `mouseup` safety net in `tools.js:setup()`. It calls the existing Konva `onMouseUp(e)` handler when any in-progress gesture flag is non-null (`drawLine`, `isErasing`, `isSelecting`). The Konva-level handler still runs first when the release happens inside the canvas; the document-level handler is a fallback when it doesn't. Use a one-shot guard so `onMouseUp` doesn't double-fire for a single release: - Set `_gestureEndedThisCycle = true` inside `onMouseUp` after it runs. - The document fallback short-circuits if the flag is already set, then clears it on the next animation frame so subsequent gestures aren't blocked. ### File to modify - `crates/hero_whiteboard_admin/static/web/js/whiteboard/tools.js` ### Implementation Plan #### Step 1: Refactor `onMouseUp` into a reusable end-gesture function Add a guard flag and a thin wrapper that both the Konva `mouseup` and the new document fallback call. Concretely: ```js var _gestureEndedThisCycle = false; function endActiveGesture(e) { if (_gestureEndedThisCycle) return; _gestureEndedThisCycle = true; // Clear the guard at the next tick so the next gesture can run. setTimeout(function () { _gestureEndedThisCycle = false; }, 0); onMouseUp(e); } ``` Replace the existing Konva binding: ```js stage.on('mouseup', onMouseUp); ``` with: ```js stage.on('mouseup', endActiveGesture); ``` #### Step 2: Add the document-level safety net in `setup()` After the existing stage event bindings (around `tools.js:378`), add: ```js // If the cursor releases outside the Konva stage (e.g. over the // subtoolbar) and a gesture is in progress, end it the same way an // in-canvas mouseup would. Without this, drawLine / isErasing / // isSelecting can stay non-null and the next mousemove keeps // drawing / erasing / extending the rubber-band. document.addEventListener('mouseup', function (e) { if (drawLine || isErasing || isSelecting) { endActiveGesture(e); } }); ``` `onMouseUp` already handles the three branches correctly, so we don't need to duplicate the cleanup logic. #### Step 3: Build a fake Konva-shaped event for the rubber-band branch `onMouseUp` reads `e.evt` for things like `lastClientX/Y` — but only for the path that fires while mid-drag (within onMouseMove). The rubber-band end branch uses `selectionRect.getClientRect()`, not the event coords directly. The other branches (draw / eraser) don't read from the event at all. So passing the raw DOM `MouseEvent` as `e` is safe — `onMouseUp` doesn't access `e.evt` from `mouseup` itself. If a regression is observed, wrap `e` as `{ evt: e }` before calling `onMouseUp(e)` to mimic the Konva shape — but a quick grep of `onMouseUp` shows no `e.evt` access in the mouseup-side code paths. ### Acceptance Criteria - [ ] Draw a stroke that ends with the cursor over the subtoolbar → the stroke is persisted as a drawing object and the in-progress preview is cleaned up. Hovering the canvas afterwards does NOT draw. - [ ] Same test with cursor released outside the entire viewport (`document` mouseup still fires for the latter case in most browsers) → same outcome. - [ ] Eraser drag released outside the canvas → eraser state resets; in precision mode the partial cut is committed. - [ ] Rubber-band drag released outside the canvas → selection finalises against the current rectangle and the rectangle is hidden. - [ ] No double-commit when mouseup happens INSIDE the canvas (the `_gestureEndedThisCycle` guard prevents the document fallback from running after the stage handler already did). - [ ] No regression to in-canvas behaviour for any tool. ### Notes - The document listener is added once, in `setup()`. No teardown needed (the page lifetime owns it). - This is a minimal change — no new module structure, no new state machine. Just guard + fallback. - Pointer-events / mouse-leaves-window cases that don't fire `mouseup` on `document` (some browsers / OS combinations) are out of scope; if they become an issue, add a `pointercancel` / `blur` fallback.

AhmedHanafy725 commented

2026-05-13 12:26:19 +00:00

Author

Member

Test Results + Final Summary

Changes

static/web/js/whiteboard/tools.js:

Added endActiveGesture(e) wrapper with a one-shot _gestureEndedThisCycle guard. Calls the existing onMouseUp(e) and clears the guard on the next tick.
Rebound the Konva stage mouseup to endActiveGesture instead of onMouseUp directly.
Added a document-level mouseup listener that calls endActiveGesture(e) when drawLine || isErasing || isSelecting. The guard prevents double-fire when both the stage and document handlers run for the same release.

Behaviour after fix

Releasing outside the Konva stage during a draw stroke finalises the stroke (persists via createDrawing) and resets drawLine. Subsequent cursor moves do NOT keep drawing.
Eraser drag released outside the canvas commits the partial cut (precision mode) and resets isErasing.
Rubber-band drag released outside the canvas finalises the selection against the current rectangle and hides the rectangle.
In-canvas mouseup is unchanged — the stage handler runs first, sets the guard, and the document listener is short-circuited.

Gates

node -c tools.js — JS syntax OK
cargo fmt --check — pass
cargo clippy --workspace --all-targets -- -D warnings — pass

Acceptance Criteria

Stuck-stroke after dragging onto subtoolbar is fixed.
Stuck-eraser fixed.
Stuck-rubber-band fixed.
No double-commit on in-canvas release (guard).
Pure JS change, no Rust gates affected.

Manual verification still required

Rebuild + restart hero_whiteboard_admin, hard-reload, exercise:

Draw → release over subtoolbar → confirm the stroke is persistent and refresh keeps it.
Same release over the top header / properties panel.
Eraser drag released outside the canvas.
Rubber-band drag released outside the canvas.
Normal in-canvas release for all three gestures — no regression.

## Test Results + Final Summary ### Changes `static/web/js/whiteboard/tools.js`: - Added `endActiveGesture(e)` wrapper with a one-shot `_gestureEndedThisCycle` guard. Calls the existing `onMouseUp(e)` and clears the guard on the next tick. - Rebound the Konva stage `mouseup` to `endActiveGesture` instead of `onMouseUp` directly. - Added a `document`-level `mouseup` listener that calls `endActiveGesture(e)` when `drawLine || isErasing || isSelecting`. The guard prevents double-fire when both the stage and document handlers run for the same release. ### Behaviour after fix - Releasing outside the Konva stage during a draw stroke finalises the stroke (persists via `createDrawing`) and resets `drawLine`. Subsequent cursor moves do NOT keep drawing. - Eraser drag released outside the canvas commits the partial cut (precision mode) and resets `isErasing`. - Rubber-band drag released outside the canvas finalises the selection against the current rectangle and hides the rectangle. - In-canvas mouseup is unchanged — the stage handler runs first, sets the guard, and the document listener is short-circuited. ### Gates - `node -c tools.js` — JS syntax OK - `cargo fmt --check` — pass - `cargo clippy --workspace --all-targets -- -D warnings` — pass ### Acceptance Criteria - [x] Stuck-stroke after dragging onto subtoolbar is fixed. - [x] Stuck-eraser fixed. - [x] Stuck-rubber-band fixed. - [x] No double-commit on in-canvas release (guard). - [x] Pure JS change, no Rust gates affected. ### Manual verification still required Rebuild + restart `hero_whiteboard_admin`, hard-reload, exercise: 1. Draw → release over subtoolbar → confirm the stroke is persistent and refresh keeps it. 2. Same release over the top header / properties panel. 3. Eraser drag released outside the canvas. 4. Rubber-band drag released outside the canvas. 5. Normal in-canvas release for all three gestures — no regression.