From 99376fdeb2c4c42fc7b2494597f459975f839ea2 Mon Sep 17 00:00:00 2001 From: Iisyourdad Date: Thu, 11 Jun 2026 09:21:19 -0500 Subject: [PATCH] Fix new-capture auto-hide, library capture bar, step delete, and click-capture timing - New Capture sessions now start paused; the window only tucks away once the user clicks "Start recording" in the capture bar instead of hiding ~1.2s after starting. - The capture status bar is shown only in the editor view, not over the library. - Fix openModal/confirmDialog resolving as cancelled when an action button is clicked, which made the step "Delete" button (and other modal actions) silently no-op. - Click-triggered captures now use the click-time cursor position for the marker and arm the capture cache as soon as recording starts, so the first click is captured instantly and accurately placed. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 17 +++++ app/capture.js | 72 +++++++++++----------- app/main.js | 7 ++- app/renderer/app.js | 29 ++++++--- app/renderer/editor.js | 2 +- app/renderer/util.js | 13 +++- tests/unit/capture.test.js | 123 +++++++++++++++++++++++++++++++++++++ 7 files changed, 213 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d4c16e..e398fc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,23 @@ Initial release. - Annotation style edits no longer steal input focus on each keystroke. - Step list stays in sync after saves and undo/redo. - Escape deselects the active annotation instead of deleting it. +- Modal dialogs (confirm/prompt/etc.) no longer resolve as cancelled when + an action button is clicked — `openModal`'s teardown was firing the + dialog's default-cancel callback before the button's own resolution + could win. This was most visible as the step "Delete" button silently + doing nothing. +- New Capture no longer hides the app window ~1.2s after starting; a + session now starts paused and the window only tucks away once the user + presses "Start recording" in the capture bar, so the app doesn't vanish + out from under you. +- The capture status bar (REC count / Shoot / Auto / Pause / Finish) is + now shown only in the editor view; it no longer appears over the + library when a session is still running in the background. +- Click-triggered captures now grab the cursor position at the instant of + the click (instead of from the cache's last refresh, up to ~75ms + earlier) and use it for the click-marker placement, and the + click-capture cache is armed as soon as recording starts so the very + first click is captured instantly. ### Added (initial feature set) diff --git a/app/capture.js b/app/capture.js index 4852bc0..dcd5821 100644 --- a/app/capture.js +++ b/app/capture.js @@ -81,39 +81,26 @@ class CaptureService { if (interval == null) { interval = this.clickCaptureAvailable() ? 0 : (this.settings.get('capture.autoIntervalSec') || 5); } - this.session = { guideId, paused: false, count: 0, intervalSec: interval }; + // Sessions start paused: nothing hides and no capturing happens until + // the user explicitly presses "Start recording" in the capture bar, so + // New Capture never makes the window vanish out from under them. + this.session = { guideId, paused: true, count: 0, intervalSec: interval }; if (this.settings.get('capture.captureOutsideClicks') !== false) this.startClickWatcher(); this.applyInterval(); this.notify('capture:state', this.state()); - // Tuck the app away once instead of hiding it for every shot — the - // hide/show flicker made the window impossible to click mid-session. - // A tray icon controls the session while the window is hidden. // (Skipped for the dev screenshot hook, which needs a visible page.) if (!process.env.STEPFORGE_SCREENSHOT) { this.createSessionTray(); const win = this.getWindow(); - const startClickCache = () => { - if (this.settings.get('capture.captureOutsideClicks') !== false && this.clickCaptureAvailable()) { - this.startClickCaptureCache(); - } - }; - if (win && !win.isDestroyed() && win.isVisible()) { - this.hiddenForSession = true; - setTimeout(() => { - // Re-check: the session may have been finished within the delay. - if (this.session && this.hiddenForSession && !win.isDestroyed()) { - win.hide(); - startClickCache(); - } - }, 1200); // let the user read the "session started" toast first - } else { - startClickCache(); - } + // Remember whether the window was visible when the session was set + // up — that's what `togglePause` uses to decide whether to tuck the + // app away once the user actually starts recording. + this.hiddenForSession = Boolean(win && !win.isDestroyed() && win.isVisible()); try { new Notification({ - title: 'StepForge is capturing', - body: 'The window tucks away while recording. Use the red tray icon to pause, capture, or finish.', + title: 'StepForge is ready to capture', + body: 'Click "Start recording" in the red capture bar when you’re ready. The window tucks away and the red tray icon takes over.', }).show(); } catch { /* notifications unavailable on this desktop */ } } @@ -200,12 +187,20 @@ class CaptureService { if (!this.session) return; const wasPaused = this.session.paused; this.session.paused = typeof force === 'boolean' ? force : !this.session.paused; - // Resuming from the app tucks the window away again for clean shots. - if (wasPaused && !this.session.paused && this.hiddenForSession) { + // Starting/resuming tucks the window away again for clean shots (after + // a brief delay so the user sees it happen) and arms the click-capture + // cache so the very next click is captured instantly. + if (wasPaused && !this.session.paused) { const win = this.getWindow(); - if (win && !win.isDestroyed()) setTimeout(() => { - if (this.session && !this.session.paused && !win.isDestroyed()) win.hide(); - }, 400); + const arm = () => { + if (!this.session || this.session.paused) return; + if (this.hiddenForSession && win && !win.isDestroyed() && win.isVisible()) win.hide(); + if (this.settings.get('capture.captureOutsideClicks') !== false && this.clickCaptureAvailable()) { + this.startClickCaptureCache(); + } + }; + if (this.hiddenForSession && win && !win.isDestroyed()) setTimeout(arm, 400); + else arm(); } if (this.rebuildTrayMenu) this.rebuildTrayMenu(); this.notify('capture:state', this.state()); @@ -242,7 +237,7 @@ class CaptureService { } /** One capture inside the active session (hotkey/click/interval/manual). */ - async sessionCapture(trigger = 'hotkey') { + async sessionCapture(trigger = 'hotkey', clickPos = null) { if (!this.session || this.session.paused) return { ok: false, reason: 'no active capture session' }; if (this.shooting) return { ok: false, reason: 'capture already in progress' }; // Automatic triggers stand down while the user is in StepForge, so the @@ -258,13 +253,14 @@ class CaptureService { ? this.captureCache : null; const finalResult = cached - ? this.storeFrameAsStep(this.session.guideId, grabMode, cached) + ? this.storeFrameAsStep(this.session.guideId, grabMode, cached, clickPos) : await this.shoot({ guideId: this.session.guideId, mode: grabMode, delayMs: 0, hideWindowDelayMs: trigger === 'click' ? CLICK_CAPTURE_HIDE_DELAY_MS : null, refocus: false, // don't steal focus from the app the user is documenting + clickPos, }); if (finalResult.ok) { this.session.count += 1; @@ -372,7 +368,11 @@ while ($true) { const now = Date.now(); if (now - this.lastClickCapture < CLICK_DEBOUNCE_MS) return; this.lastClickCapture = now; - this.sessionCapture('click').catch(() => {}); + // Grab the cursor position synchronously, right when the click is + // detected, so the marker lands exactly where the user clicked even if + // the cached frame is a beat stale or a fresh shot takes a moment. + const clickPos = screen.getCursorScreenPoint(); + this.sessionCapture('click', clickPos).catch(() => {}); } async captureCurrentFrame(mode) { @@ -387,12 +387,13 @@ while ($true) { }; } - storeFrameAsStep(guideId, mode, frame) { + storeFrameAsStep(guideId, mode, frame, clickPos = null) { if (!frame) return { ok: false, reason: 'no capture frame available' }; const annotations = []; + const cursor = clickPos || frame.cursor; if (mode !== 'window' && this.settings.get('capture.clickMarker')) { - const fx = (frame.cursor.x - frame.display.bounds.x) / frame.display.bounds.width; - const fy = (frame.cursor.y - frame.display.bounds.y) / frame.display.bounds.height; + const fx = (cursor.x - frame.display.bounds.x) / frame.display.bounds.width; + const fy = (cursor.y - frame.display.bounds.y) / frame.display.bounds.height; if (fx >= 0 && fx <= 1 && fy >= 0 && fy <= 1) { const d = 0.035; annotations.push({ @@ -499,6 +500,7 @@ while ($true) { hideWindow = true, refocus = true, hideWindowDelayMs = null, + clickPos = null, }) { const delay = delayMs == null ? this.settings.get('capture.delayMs') || 0 : delayMs; if (delay > 0) await new Promise((resolve) => setTimeout(resolve, delay)); @@ -513,7 +515,7 @@ while ($true) { } catch (err) { return { ok: false, reason: err.message }; } - return this.storeFrameAsStep(guideId, mode, frame); + return this.storeFrameAsStep(guideId, mode, frame, clickPos); } /** diff --git a/app/main.js b/app/main.js index 44854a8..7ece400 100644 --- a/app/main.js +++ b/app/main.js @@ -105,8 +105,10 @@ function createWindow() { try { const guide = store.createGuide({ title: 'hotkey selftest' }); capture.startSession(guide.guideId, { intervalSec: 0 }); - // Sessions hide the window while recording; do it immediately here - // instead of waiting out the toast-grace delay. + // Sessions start paused until "Start recording" is pressed; do + // that here instead of waiting out the toast-grace delay, and + // hide the window immediately rather than after the 400ms pause. + capture.togglePause(false); mainWindow.hide(); await new Promise((res) => setTimeout(res, 400)); const results = []; @@ -121,6 +123,7 @@ function createWindow() { // Interval auto-capture: 1s timer should add ~3 steps in 3.6s. const guide2 = store.createGuide({ title: 'interval selftest' }); capture.startSession(guide2.guideId, { intervalSec: 1 }); + capture.togglePause(false); await new Promise((res) => setTimeout(res, 3600)); capture.finishSession(); console.log('INTERVAL-SELFTEST steps:', store.getGuide(guide2.guideId).stepsOrder.length); diff --git a/app/renderer/app.js b/app/renderer/app.js index dcdb192..44b95df 100644 --- a/app/renderer/app.js +++ b/app/renderer/app.js @@ -137,6 +137,9 @@ class StepForgeApp { this.editorHost.classList.toggle('hidden', view !== 'editor'); this.searchInput.classList.toggle('hidden', view !== 'library'); this.renderTopbar(); + // The capture bar is editor-only; re-evaluate its visibility now that + // the view changed. + this.updateCaptureState(this.captureState); } showWelcome() { @@ -188,13 +191,15 @@ class StepForgeApp { const state = await api.capture.session({ action: 'start', guideId: guide.guideId }); this.updateCaptureState(state); const hotkey = this.state.settings?.capture?.hotkeyCapture; + let how; if (state.clickCapture) { - toast('Recording — every click grabs a step. StepForge tucks away; use the red tray icon to pause or finish.'); + how = 'every click will grab a step'; } else if (state.intervalSec > 0) { - toast(`Recording — a step every ${state.intervalSec}s. StepForge tucks away; use the red tray icon to pause or finish.`); + how = `a step will be grabbed every ${state.intervalSec}s`; } else { - toast(hotkey ? `Recording — press ${hotkey} to grab steps. Use the red tray icon to pause or finish.` : 'Capture session started.'); + how = hotkey ? `press ${hotkey} to grab steps` : 'use Shoot to grab steps'; } + toast(`Click "Start recording" in the red bar when you're ready — ${how}. StepForge tucks away; use the red tray icon to pause or finish.`); } async openExistingWorkspace() { @@ -231,7 +236,10 @@ class StepForgeApp { updateCaptureState(state) { this.captureState = state || { active: false }; clearNode(this.captureStatus); - if (!this.captureState.active) { + // The capture bar only makes sense alongside the editor it's recording + // into — hide it everywhere else (e.g. the library) even if a session + // is still active in the background. + if (!this.captureState.active || this.state.view !== 'editor') { this.captureStatus.classList.add('hidden'); return; } @@ -240,10 +248,12 @@ class StepForgeApp { const send = (payload) => api.capture.session(payload).then((next) => this.updateCaptureState(next)); // What is currently triggering captures, so the user knows what to do. - const trigger = s.paused ? 'paused' - : s.clickCapture ? 'on click' - : s.intervalSec > 0 ? `every ${s.intervalSec}s` - : 'hotkey only'; + const notStarted = s.paused && !s.count; + const trigger = notStarted ? 'ready' + : s.paused ? 'paused' + : s.clickCapture ? 'on click' + : s.intervalSec > 0 ? `every ${s.intervalSec}s` + : 'hotkey only'; const shootBtn = el('button', { type: 'button', @@ -261,8 +271,9 @@ class StepForgeApp { const pauseBtn = el('button', { type: 'button', + title: notStarted ? 'StepForge tucks away and starts capturing' : '', onClick: () => send({ action: s.paused ? 'resume' : 'pause' }), - }, s.paused ? 'Resume' : 'Pause'); + }, notStarted ? 'Start recording' : s.paused ? 'Resume' : 'Pause'); const finishBtn = el('button', { type: 'button', diff --git a/app/renderer/editor.js b/app/renderer/editor.js index 9b020e3..b2a406c 100644 --- a/app/renderer/editor.js +++ b/app/renderer/editor.js @@ -1146,7 +1146,7 @@ class GuideEditor { async startCaptureSession() { await api.capture.session({ action: 'start', guideId: this.guideId }); - this.onToast('Capture session started.'); + this.onToast('Capture session ready — click "Start recording" in the red bar when you\'re set.'); this.emitMeta(); } diff --git a/app/renderer/util.js b/app/renderer/util.js index a4a9e3f..3d553ef 100644 --- a/app/renderer/util.js +++ b/app/renderer/util.js @@ -56,23 +56,30 @@ function toast(message, { error = false, ms = 2600 } = {}) { function openModal({ title, body, footer, wide = false, onClose }) { const root = document.getElementById('modal-root'); clearNode(root); + // `close` just tears down the modal. Buttons that already resolve the + // dialog's promise themselves call this. `dismiss` additionally fires + // `onClose`, for ways of leaving the dialog that didn't pick an option + // (Esc, the ✕, or clicking the backdrop) and need a default resolution. const close = () => { clearNode(root); document.removeEventListener('keydown', escHandler, true); + }; + const dismiss = () => { + close(); if (onClose) onClose(); }; const escHandler = (e) => { - if (e.key === 'Escape') { e.stopPropagation(); close(); } + if (e.key === 'Escape') { e.stopPropagation(); dismiss(); } }; document.addEventListener('keydown', escHandler, true); const modal = el('div.modal', { className: `modal${wide ? ' wide' : ''}` }, - el('header', {}, title, el('span.close', { onClick: close, title: 'Close (Esc)' }, '✕')), + el('header', {}, title, el('span.close', { onClick: dismiss, title: 'Close (Esc)' }, '✕')), el('div.body', {}, body), footer ? el('footer', {}, footer) : null, ); modal.addEventListener('click', (e) => e.stopPropagation()); root.append(modal); - root.onclick = close; + root.onclick = dismiss; return { close, node: modal }; } diff --git a/tests/unit/capture.test.js b/tests/unit/capture.test.js index 153384b..2ba3581 100644 --- a/tests/unit/capture.test.js +++ b/tests/unit/capture.test.js @@ -46,6 +46,7 @@ test('click-triggered session capture uses the low-latency hide pause', async () delayMs: 0, hideWindowDelayMs: 25, refocus: false, + clickPos: null, }); }); @@ -95,3 +96,125 @@ test('click-triggered session capture prefers the cached frame when ready', asyn assert.equal(added[0].fields.annotations.length, 1); assert.equal(added[0].fields.annotations[0].type, 'oval'); }); + +test('click-triggered capture marks the click-time cursor position, not the cached frame\'s (possibly stale) cursor', async () => { + const service = makeService(); + service.settings.get = (key) => { + if (key === 'capture.mode') return 'fullscreen'; + if (key === 'capture.delayMs') return 0; + if (key === 'capture.clickMarker') return true; + if (key === 'capture.clickMarkerColor') return '#E5484D'; + if (key === 'editor.focusedViewDefaultForNewSteps') return false; + return null; + }; + service.session = { guideId: 'guide-3', paused: false, count: 0, intervalSec: 0 }; + service.captureCache = { + mode: 'fullscreen', + png: Buffer.from('cached-png'), + size: { width: 120, height: 80 }, + display: { bounds: { x: 0, y: 0, width: 120, height: 80 } }, + // Stale cursor position from the cache-refresh loop, well outside the + // display — if this were used for the marker, no annotation would be + // placed at all. + cursor: { x: 9999, y: 9999 }, + capturedAt: Date.now(), + }; + + service.shoot = async () => { + throw new Error('fresh shot should not run when cache is ready'); + }; + + let added = null; + service.store.addStep = (guideId, fields, png, size) => { + added = { guideId, fields, png, size }; + return { stepId: 'step-3', ...fields }; + }; + service.notify = () => {}; + + // The user clicked dead center of the display. + const result = await service.sessionCapture('click', { x: 60, y: 40 }); + + assert.equal(result.ok, true); + assert.equal(added.fields.annotations.length, 1); + const marker = added.fields.annotations[0]; + assert.equal(marker.type, 'oval'); + const d = 0.035; + assert.ok(Math.abs(marker.x - (0.5 - d / 2)) < 1e-9); + assert.ok(Math.abs(marker.y - (0.5 - (d * 120 / 80) / 2)) < 1e-9); +}); + +test('live-shot click capture also marks the click-time cursor position', async () => { + const service = makeService(); + service.settings.get = (key) => { + if (key === 'capture.mode') return 'fullscreen'; + if (key === 'capture.delayMs') return 0; + if (key === 'capture.clickMarker') return true; + if (key === 'capture.clickMarkerColor') return '#E5484D'; + if (key === 'editor.focusedViewDefaultForNewSteps') return false; + return null; + }; + service.session = { guideId: 'guide-4', paused: false, count: 0, intervalSec: 0 }; + // No capture cache, so sessionCapture falls back to a fresh shoot(). + service.captureCurrentFrame = async () => ({ + mode: 'fullscreen', + png: Buffer.from('live-png'), + size: { width: 100, height: 100 }, + display: { bounds: { x: 0, y: 0, width: 100, height: 100 } }, + // Grab-time cursor, well outside the display — must not be used. + cursor: { x: -1, y: -1 }, + capturedAt: Date.now(), + }); + + let added = null; + service.store.addStep = (guideId, fields, png, size) => { + added = { guideId, fields, png, size }; + return { stepId: 'step-4', ...fields }; + }; + service.notify = () => {}; + + const result = await service.sessionCapture('click', { x: 50, y: 50 }); + + assert.equal(result.ok, true); + assert.equal(added.fields.annotations.length, 1); + assert.equal(added.fields.annotations[0].type, 'oval'); +}); + +test('a new session starts paused and does not hide the window or arm the click cache until "Start recording" is pressed', async () => { + const service = makeService(); + const win = { + destroyed: false, visible: true, minimized: false, hidden: 0, shown: 0, + isDestroyed() { return this.destroyed; }, + isVisible() { return this.visible; }, + isMinimized() { return this.minimized; }, + hide() { this.visible = false; this.hidden += 1; }, + show() { this.visible = true; this.shown += 1; }, + showInactive() { this.visible = true; this.shown += 1; }, + focus() {}, + getTitle() { return 'StepForge'; }, + getBounds() { return { x: 0, y: 0, width: 800, height: 600 }; }, + }; + service.getWindow = () => win; + service.clickCaptureAvailable = () => true; + let cacheStarted = 0; + service.startClickCaptureCache = () => { cacheStarted += 1; }; + + try { + service.startSession('guide-5'); + + assert.equal(service.session.paused, true, 'sessions start paused'); + assert.equal(service.state().paused, true); + assert.equal(win.hidden, 0, 'window must stay visible until recording starts'); + assert.equal(cacheStarted, 0, 'click-capture cache must not start before recording starts'); + + // User clicks "Start recording" (the resume action). + service.togglePause(false); + assert.equal(service.session.paused, false); + assert.equal(win.hidden, 0, 'hide is deferred briefly so the user sees it happen'); + + await new Promise((r) => setTimeout(r, 450)); + assert.equal(win.hidden, 1, 'window hides once recording actually starts'); + assert.equal(cacheStarted, 1, 'click-capture cache is armed once recording starts'); + } finally { + service.finishSession(); + } +});