diff --git a/static/js/managers/BulkManager.js b/static/js/managers/BulkManager.js index a0917acb..0eecac6c 100644 --- a/static/js/managers/BulkManager.js +++ b/static/js/managers/BulkManager.js @@ -27,6 +27,8 @@ export class BulkManager { // Drag detection properties this.dragThreshold = 5; // Pixels to move before considering it a drag + this.dragDelayMs = 100; // Minimum hold time before a drag is treated as a marquee + this.minMarqueeSize = 10; // Minimum drag box (px) before a marquee counts as a selection this.mouseDownTime = 0; this.mouseDownPosition = { x: 0, y: 0 }; @@ -173,6 +175,19 @@ export class BulkManager { }); eventManager.addHandler('mousemove', 'bulkManager-marquee-move', (e) => { + // Only track marquee/drag while the left button is physically held. + // mouseup can be missed (release outside the window, focus loss, driver quirks), + // so mousemove must verify the button state itself instead of relying on it. + if (!(e.buttons & 1)) { + if (this.isMarqueeActive) { + this.endMarqueeSelection(e); + } else { + this.mouseDownTime = 0; + this.isDragging = false; + } + return false; + } + if (this.isMarqueeActive) { this.lastClientX = e.clientX; this.lastClientY = e.clientY; @@ -184,7 +199,10 @@ export class BulkManager { const dy = e.clientY - this.mouseDownPosition.y; const distance = Math.sqrt(dx * dx + dy * dy); - if (distance >= this.dragThreshold) { + // Require both enough movement AND enough hold time so quick + // click jitter from micro-movement input devices is not a marquee. + const heldTime = Date.now() - this.mouseDownTime; + if (heldTime >= this.dragDelayMs && distance >= this.dragThreshold) { this.isDragging = true; this.startMarqueeSelection(e, true); } @@ -1958,9 +1976,31 @@ export class BulkManager { // Remove visual feedback class document.body.classList.remove('marquee-selecting'); + // Compute the actual drag box size in document coordinates, matching how + // updateMarqueeSelectionFromPosition tracks the rectangle. Client-space + // size would wrongly flag auto-scroll marquees (tiny pointer movement, + // large document-space box) as accidental clicks. + const container = document.querySelector('.page-content'); + const scrollX = container?.scrollLeft || 0; + const scrollY = container?.scrollTop || 0; + const dragWidth = Math.abs((e.clientX + scrollX) - this.marqueeStartDoc.x); + const dragHeight = Math.abs((e.clientY + scrollY) - this.marqueeStartDoc.y); + const isTinyMarquee = dragWidth < this.minMarqueeSize && dragHeight < this.minMarqueeSize; + // Get selection count const selectionCount = state.selectedModels.size; + // A tiny box (e.g. click jitter that happened to graze a card) is treated + // as an accidental click: undo any selection and leave bulk mode. + if (isTinyMarquee) { + this.clearSelection(); + if (state.bulkMode) { + this.toggleBulkMode(); + } + this.initialSelectedModels.clear(); + return; + } + // If no models were selected, exit bulk mode if (selectionCount === 0) { if (state.bulkMode) { diff --git a/tests/frontend/managers/BulkManager.marquee.test.js b/tests/frontend/managers/BulkManager.marquee.test.js new file mode 100644 index 00000000..ff1ed68d --- /dev/null +++ b/tests/frontend/managers/BulkManager.marquee.test.js @@ -0,0 +1,186 @@ +import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; +import { state } from '../../../static/js/state/index.js'; +import { MODEL_TYPES } from '../../../static/js/api/apiConfig.js'; +import { eventManager } from '../../../static/js/utils/EventManager.js'; +import { BulkManager } from '../../../static/js/managers/BulkManager.js'; + +function fire(type, init = {}) { + return new MouseEvent(type, { bubbles: true, cancelable: true, ...init }); +} + +describe('BulkManager marquee guards', () => { + beforeEach(() => { + vi.useFakeTimers(); + // jsdom may not provide requestAnimationFrame; stub it so the auto-scroll loop is a no-op. + window.requestAnimationFrame = vi.fn(); + window.cancelAnimationFrame = vi.fn(); + + eventManager.cleanup(); + state.currentPageType = MODEL_TYPES.LORA; + state.bulkMode = false; + state.selectedModels.clear(); + + document.body.innerHTML = '
'; + const pageContent = document.querySelector('.page-content'); + pageContent.getBoundingClientRect = () => ({ + top: 0, + left: 0, + right: 1000, + bottom: 1000, + width: 1000, + height: 1000, + x: 0, + y: 0, + toJSON: () => ({}), + }); + pageContent.scrollBy = vi.fn(); + }); + + afterEach(() => { + eventManager.cleanup(); + vi.useRealTimers(); + document.body.innerHTML = ''; + }); + + function createBulkManager() { + const bulk = new BulkManager(); + bulk.initialize(); + return bulk; + } + + it('never starts a marquee when the left button is not held', () => { + const bulk = createBulkManager(); + const pageContent = document.querySelector('.page-content'); + + pageContent.dispatchEvent(fire('mousedown', { button: 0, clientX: 10, clientY: 10 })); + document.dispatchEvent(fire('mousemove', { buttons: 0, clientX: 50, clientY: 50 })); + + expect(bulk.mouseDownTime).toBe(0); + expect(bulk.isMarqueeActive).toBe(false); + expect(state.bulkMode).toBe(false); + expect(document.querySelector('.marquee-selection')).toBeNull(); + }); + + it('requires holding the left button for the drag delay before starting a marquee', () => { + const bulk = createBulkManager(); + const pageContent = document.querySelector('.page-content'); + + pageContent.dispatchEvent(fire('mousedown', { button: 0, clientX: 10, clientY: 10 })); + + // Fast movement: far enough, but too soon after mousedown. + document.dispatchEvent(fire('mousemove', { buttons: 1, clientX: 30, clientY: 10 })); + expect(state.bulkMode).toBe(false); + expect(bulk.isMarqueeActive).toBe(false); + + // Once the hold time has elapsed, the same drag qualifies. + vi.advanceTimersByTime(100); + document.dispatchEvent(fire('mousemove', { buttons: 1, clientX: 35, clientY: 12 })); + expect(state.bulkMode).toBe(true); + expect(bulk.isMarqueeActive).toBe(true); + expect(document.querySelector('.marquee-selection')).not.toBeNull(); + }); + + it('ends an active marquee if the left button is released without a mouseup event', () => { + const bulk = createBulkManager(); + bulk.mouseDownPosition = { x: 10, y: 10 }; + bulk.startMarqueeSelection({}, true); + expect(state.bulkMode).toBe(true); + expect(document.querySelector('.marquee-selection')).not.toBeNull(); + + // No mouseup was dispatched; a plain move with the button released finalizes it. + document.dispatchEvent(fire('mousemove', { buttons: 0, clientX: 50, clientY: 50 })); + + expect(bulk.isMarqueeActive).toBe(false); + expect(document.querySelector('.marquee-selection')).toBeNull(); + expect(state.bulkMode).toBe(false); // zero selected -> auto-exit + }); + + it('treats a tiny marquee as an accidental click: clears selection and exits bulk mode', () => { + const bulk = createBulkManager(); + const card = document.createElement('div'); + card.className = 'model-card selected'; + card.dataset.filepath = '/models/test.safetensors'; + document.body.appendChild(card); + state.selectedModels.add('/models/test.safetensors'); + + bulk.mouseDownPosition = { x: 100, y: 100 }; + bulk.startMarqueeSelection({}, true); + expect(state.bulkMode).toBe(true); + + bulk.endMarqueeSelection({ clientX: 103, clientY: 104 }); + + expect(state.bulkMode).toBe(false); + expect(state.selectedModels.size).toBe(0); + expect(card.classList.contains('selected')).toBe(false); + }); + + it('keeps selection and bulk mode when the marquee is large enough', () => { + const bulk = createBulkManager(); + const card = document.createElement('div'); + card.className = 'model-card selected'; + card.dataset.filepath = '/models/test.safetensors'; + document.body.appendChild(card); + state.selectedModels.add('/models/test.safetensors'); + + bulk.mouseDownPosition = { x: 100, y: 100 }; + bulk.startMarqueeSelection({}, true); + + bulk.endMarqueeSelection({ clientX: 130, clientY: 140 }); + + expect(state.bulkMode).toBe(true); + expect(state.selectedModels.has('/models/test.safetensors')).toBe(true); + expect(card.classList.contains('selected')).toBe(true); + }); + + it('keeps auto-scroll marquee selections when the pointer only moved a few pixels', () => { + const bulk = createBulkManager(); + const pageContent = document.querySelector('.page-content'); + + // Card just below the press point in document coordinates. + const card = document.createElement('div'); + card.className = 'model-card'; + card.dataset.filepath = '/models/off-screen.safetensors'; + card.getBoundingClientRect = () => ({ + top: 950, + left: 400, + right: 600, + bottom: 1050, + width: 200, + height: 100, + x: 400, + y: 950, + toJSON: () => ({}), + }); + document.body.appendChild(card); + + pageContent.dispatchEvent(fire('mousedown', { button: 0, clientX: 500, clientY: 900 })); + vi.advanceTimersByTime(100); + + // Small pointer move: enough to start the marquee, but under minMarqueeSize. + document.dispatchEvent(fire('mousemove', { buttons: 1, clientX: 506, clientY: 906 })); + expect(bulk.isMarqueeActive).toBe(true); + + // Auto-scroll grows the document-space box while the pointer stays nearly still. + pageContent.scrollTop = 200; + card.getBoundingClientRect = () => ({ + top: 750, + left: 400, + right: 600, + bottom: 850, + width: 200, + height: 100, + x: 400, + y: 750, + toJSON: () => ({}), + }); + document.dispatchEvent(fire('mousemove', { buttons: 1, clientX: 506, clientY: 906 })); + + expect(state.selectedModels.has('/models/off-screen.safetensors')).toBe(true); + + // Release: the client-space box is tiny, but the document-space box is not. + document.dispatchEvent(fire('mouseup', { button: 0, clientX: 506, clientY: 906 })); + + expect(state.selectedModels.has('/models/off-screen.safetensors')).toBe(true); + expect(state.bulkMode).toBe(true); + }); +});