From 1672dce80377d8f613707fe2cf2f9563e0d8b264 Mon Sep 17 00:00:00 2001 From: NimBold Date: Sat, 22 Aug 2026 01:05:36 +0330 Subject: [PATCH] fix(table): harden queue and row interaction lifecycles - fence column resize and queue lost-capture handlers to the active pointer - keep staged bulk actions and RTL submenu keyboard navigation truthful - add deterministic selection, resize, action-count, and keyboard regressions --- src/components/DownloadTable.tsx | 56 ++++++++-------- src/components/FloatingQueueSubmenu.tsx | 4 +- src/utils/columnResize.test.ts | 85 +++++++++++++++++++++++++ src/utils/columnResize.ts | 61 ++++++++++++++++++ src/utils/downloadActions.test.ts | 2 +- src/utils/downloadActions.ts | 3 +- src/utils/downloadSelection.test.ts | 16 ++++- src/utils/downloadSelection.ts | 11 ++++ src/utils/floatingPosition.test.ts | 10 ++- src/utils/floatingPosition.ts | 3 + 10 files changed, 216 insertions(+), 35 deletions(-) create mode 100644 src/utils/columnResize.test.ts create mode 100644 src/utils/columnResize.ts diff --git a/src/components/DownloadTable.tsx b/src/components/DownloadTable.tsx index e889497..f013833 100644 --- a/src/components/DownloadTable.tsx +++ b/src/components/DownloadTable.tsx @@ -53,7 +53,8 @@ import { import { moveSelectedBlockToIndex } from '../utils/queueOrdering'; -import { updateDownloadSelection } from '../utils/downloadSelection'; +import { selectContextMenuTarget, updateDownloadSelection } from '../utils/downloadSelection'; +import { createColumnResizeSession } from '../utils/columnResize'; import { clampFloatingPosition } from '../utils/floatingPosition'; import { FloatingQueueSubmenu } from './FloatingQueueSubmenu'; import { openPropertiesWindow } from '../propertiesBridge'; @@ -671,33 +672,27 @@ export const DownloadTable: React.FC = ({ filter, onSummaryC const startX = event.clientX; const startWidth = columnWidthsRef.current[index]; - const handlePointerMove = (moveEvent: PointerEvent) => { - const nextWidth = Math.max(COLUMN_MINIMUMS[index], startWidth + moveEvent.clientX - startX); + const cleanup = createColumnResizeSession({ + windowTarget: window, + documentTarget: document, + body: document.body, + pointerId: event.pointerId, + startX, + startWidth, + minWidth: COLUMN_MINIMUMS[index], + onWidth: nextWidth => { const nextWidths = columnWidthsRef.current.map((width, columnIndex) => columnIndex === index ? nextWidth : width ); columnWidthsRef.current = nextWidths; setColumnWidths(nextWidths); - }; - - const handlePointerUp = () => { - window.removeEventListener('pointermove', handlePointerMove); - window.removeEventListener('pointerup', handlePointerUp); - window.removeEventListener('pointercancel', handlePointerUp); - window.removeEventListener('blur', handlePointerUp); - document.removeEventListener('visibilitychange', handlePointerUp); - persistColumnWidths(columnWidthsRef.current); - document.body.classList.remove('is-column-resizing'); - resizeCleanupRef.current = null; - }; - - resizeCleanupRef.current = handlePointerUp; - document.body.classList.add('is-column-resizing'); - window.addEventListener('pointermove', handlePointerMove); - window.addEventListener('pointerup', handlePointerUp); - window.addEventListener('pointercancel', handlePointerUp); - window.addEventListener('blur', handlePointerUp); - document.addEventListener('visibilitychange', handlePointerUp); + }, + onEnd: () => { + persistColumnWidths(columnWidthsRef.current); + resizeCleanupRef.current = null; + }, + }); + resizeCleanupRef.current = cleanup; }; const clampMenuPosition = useCallback((x: number, y: number, menuWidth: number, menuHeight: number) => { @@ -1335,7 +1330,9 @@ export const DownloadTable: React.FC = ({ filter, onSummaryC finishQueueDrag(true); } }; - const lostPointerCapture = () => finishQueueDrag(true); + const lostPointerCapture = (event: Event) => { + if ((event as PointerEvent).pointerId === pointerId) finishQueueDrag(true); + }; const cancel = () => finishQueueDrag(true); window.addEventListener('pointermove', pointerMove); window.addEventListener('pointerup', pointerUp); @@ -1779,10 +1776,13 @@ export const DownloadTable: React.FC = ({ filter, onSummaryC }, [clearQueueClickSuppression, handleDownloadDoubleClick]); const handleContextMenu = useCallback((menu: { x: number; y: number; id: string }) => { - if (!selectedIdsRef.current.has(menu.id)) { - setSelectedIds(new Set([menu.id])); - setLastSelectedId(menu.id); - } + const nextSelection = selectContextMenuTarget({ + selectedIds: selectedIdsRef.current, + lastSelectedId: lastSelectedIdRef.current, + targetId: menu.id, + }); + setSelectedIds(nextSelection.selectedIds); + setLastSelectedId(nextSelection.lastSelectedId); setColumnMenu(null); const position = clampMenuPosition(menu.x, menu.y, 200, 300); setContextMenuPosition(position); diff --git a/src/components/FloatingQueueSubmenu.tsx b/src/components/FloatingQueueSubmenu.tsx index 87a35c6..86182e4 100644 --- a/src/components/FloatingQueueSubmenu.tsx +++ b/src/components/FloatingQueueSubmenu.tsx @@ -2,7 +2,7 @@ import React, { useCallback, useLayoutEffect, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; import { ChevronRight } from 'lucide-react'; import type { Queue } from '../store/useDownloadStore'; -import { positionFloatingSubmenu, type FloatingSubmenuPosition } from '../utils/floatingPosition'; +import { isFloatingSubmenuCloseKey, positionFloatingSubmenu, type FloatingSubmenuPosition } from '../utils/floatingPosition'; interface FloatingQueueSubmenuProps { label: React.ReactNode; @@ -167,7 +167,7 @@ export const FloatingQueueSubmenu: React.FC = ({ labe } }} onKeyDown={event => { - if (event.key !== 'Escape' && event.key !== 'ArrowLeft') return; + if (!isFloatingSubmenuCloseKey(event.key, isRtl)) return; event.preventDefault(); event.stopPropagation(); closeMenu(); diff --git a/src/utils/columnResize.test.ts b/src/utils/columnResize.test.ts new file mode 100644 index 0000000..f7efe81 --- /dev/null +++ b/src/utils/columnResize.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createColumnResizeSession } from './columnResize'; + +const createEventTarget = () => { + const listeners = new Map>(); + const target = { + addEventListener: vi.fn((type: string, listener: EventListener) => { + const current = listeners.get(type) ?? new Set(); + current.add(listener); + listeners.set(type, current); + }), + removeEventListener: vi.fn((type: string, listener: EventListener) => { + listeners.get(type)?.delete(listener); + }), + }; + return { + target, + dispatch: (type: string, event: Partial = {}) => { + listeners.get(type)?.forEach(listener => listener(event as Event)); + }, + }; +}; + +describe('column resize session', () => { + it('ignores other pointers, clamps the active pointer, and persists on completion', () => { + const windowTarget = createEventTarget(); + const documentTarget = createEventTarget(); + const classes = new Set(); + const onWidth = vi.fn(); + const onEnd = vi.fn(); + const classList = { + add: (value: string) => classes.add(value), + remove: (value: string) => classes.delete(value), + } as unknown as DOMTokenList; + createColumnResizeSession({ + windowTarget: windowTarget.target, + documentTarget: documentTarget.target, + body: { classList }, + pointerId: 7, + startX: 100, + startWidth: 220, + minWidth: 92, + onWidth, + onEnd, + }); + + expect(classes.has('is-column-resizing')).toBe(true); + windowTarget.dispatch('pointermove', { pointerId: 8, clientX: 1 }); + expect(onWidth).not.toHaveBeenCalled(); + windowTarget.dispatch('pointermove', { pointerId: 7, clientX: 1 }); + expect(onWidth).toHaveBeenLastCalledWith(121); + windowTarget.dispatch('pointermove', { pointerId: 7, clientX: 1000 }); + expect(onWidth).toHaveBeenLastCalledWith(1120); + + windowTarget.dispatch('pointerup', { pointerId: 8 }); + expect(classes.has('is-column-resizing')).toBe(true); + windowTarget.dispatch('pointerup', { pointerId: 7 }); + expect(classes.has('is-column-resizing')).toBe(false); + expect(onEnd).toHaveBeenCalledTimes(1); + }); + + it('cleans up on visibility interruption and makes cleanup idempotent', () => { + const windowTarget = createEventTarget(); + const documentTarget = createEventTarget(); + const classList = { + add: vi.fn(), + remove: vi.fn(), + } as unknown as DOMTokenList; + const cleanup = createColumnResizeSession({ + windowTarget: windowTarget.target, + documentTarget: documentTarget.target, + body: { classList }, + pointerId: 3, + startX: 100, + startWidth: 220, + minWidth: 92, + onWidth: vi.fn(), + }); + + documentTarget.dispatch('visibilitychange'); + expect(classList.remove).toHaveBeenCalledWith('is-column-resizing'); + cleanup(); + expect(classList.remove).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/utils/columnResize.ts b/src/utils/columnResize.ts new file mode 100644 index 0000000..da4338a --- /dev/null +++ b/src/utils/columnResize.ts @@ -0,0 +1,61 @@ +type ResizeEventTarget = Pick; +type ResizeDocumentTarget = Pick; +type ResizeBody = Pick; + +/** Own the global listeners for one table-column resize gesture. */ +export const createColumnResizeSession = ({ + windowTarget, + documentTarget, + body, + pointerId, + startX, + startWidth, + minWidth, + onWidth, + onEnd, +}: { + windowTarget: ResizeEventTarget; + documentTarget: ResizeDocumentTarget; + body: ResizeBody; + pointerId: number; + startX: number; + startWidth: number; + minWidth: number; + onWidth: (width: number) => void; + onEnd?: () => void; +}): (() => void) => { + let active = true; + + const handlePointerMove = (event: Event) => { + const pointerEvent = event as PointerEvent; + if (!active || pointerEvent.pointerId !== pointerId) return; + onWidth(Math.max(minWidth, startWidth + pointerEvent.clientX - startX)); + }; + + const cleanup = () => { + if (!active) return; + active = false; + windowTarget.removeEventListener('pointermove', handlePointerMove); + windowTarget.removeEventListener('pointerup', handlePointerEnd); + windowTarget.removeEventListener('pointercancel', handlePointerEnd); + windowTarget.removeEventListener('blur', handleInterrupted); + documentTarget.removeEventListener('visibilitychange', handleInterrupted); + body.classList.remove('is-column-resizing'); + onEnd?.(); + }; + + const handlePointerEnd = (event: Event) => { + if ((event as PointerEvent).pointerId === pointerId) cleanup(); + }; + + const handleInterrupted = () => cleanup(); + + body.classList.add('is-column-resizing'); + windowTarget.addEventListener('pointermove', handlePointerMove); + windowTarget.addEventListener('pointerup', handlePointerEnd); + windowTarget.addEventListener('pointercancel', handlePointerEnd); + windowTarget.addEventListener('blur', handleInterrupted); + documentTarget.addEventListener('visibilitychange', handleInterrupted); + + return cleanup; +}; diff --git a/src/utils/downloadActions.test.ts b/src/utils/downloadActions.test.ts index a4ecfb7..3964d14 100644 --- a/src/utils/downloadActions.test.ts +++ b/src/utils/downloadActions.test.ts @@ -72,7 +72,7 @@ describe('download action policy', () => { { status: 'completed' }, ]); - expect(counts).toEqual({ pause: 3, resume: 3 }); + expect(counts).toEqual({ pause: 3, resume: 4 }); }); it('keeps large action badges compact without changing the accessible count', () => { diff --git a/src/utils/downloadActions.ts b/src/utils/downloadActions.ts index 3b685fb..083d030 100644 --- a/src/utils/downloadActions.ts +++ b/src/utils/downloadActions.ts @@ -45,8 +45,7 @@ export const countDownloadActions = ( downloads: ReadonlyArray<{ status: DownloadStatus }> ): DownloadActionCounts => downloads.reduce((counts, download) => { if (canPauseDownload(download.status)) counts.pause += 1; - if (download.status === 'paused' - || (canStartDownload(download.status) && !canPauseDownload(download.status))) { + if (canStartDownload(download.status)) { counts.resume += 1; } return counts; diff --git a/src/utils/downloadSelection.test.ts b/src/utils/downloadSelection.test.ts index 2db5285..8a94e30 100644 --- a/src/utils/downloadSelection.test.ts +++ b/src/utils/downloadSelection.test.ts @@ -1,9 +1,23 @@ import { describe, expect, it } from 'vitest'; -import { updateDownloadSelection } from './downloadSelection'; +import { selectContextMenuTarget, updateDownloadSelection } from './downloadSelection'; const orderedIds = ['a', 'b', 'c', 'd']; describe('download selection', () => { + it('selects an unselected context-menu target without losing an existing selected target', () => { + expect(selectContextMenuTarget({ + selectedIds: new Set(['a', 'b']), + lastSelectedId: 'b', + targetId: 'c', + })).toEqual({ selectedIds: new Set(['c']), lastSelectedId: 'c' }); + + expect(selectContextMenuTarget({ + selectedIds: new Set(['a', 'b']), + lastSelectedId: 'b', + targetId: 'b', + })).toEqual({ selectedIds: new Set(['a', 'b']), lastSelectedId: 'b' }); + }); + it('collapses select-all to the clicked row', () => { const result = updateDownloadSelection({ orderedIds, diff --git a/src/utils/downloadSelection.ts b/src/utils/downloadSelection.ts index 381289a..a42057e 100644 --- a/src/utils/downloadSelection.ts +++ b/src/utils/downloadSelection.ts @@ -3,6 +3,17 @@ export interface DownloadSelectionResult { lastSelectedId: string | null; } +export const selectContextMenuTarget = ({ + selectedIds, + lastSelectedId, + targetId, +}: Pick & { targetId: string }): DownloadSelectionResult => { + if (selectedIds.has(targetId)) { + return { selectedIds: new Set(selectedIds), lastSelectedId }; + } + return { selectedIds: new Set([targetId]), lastSelectedId: targetId }; +}; + interface DownloadSelectionOptions { orderedIds: readonly string[]; selectedIds: ReadonlySet; diff --git a/src/utils/floatingPosition.test.ts b/src/utils/floatingPosition.test.ts index d281f46..d470bbb 100644 --- a/src/utils/floatingPosition.test.ts +++ b/src/utils/floatingPosition.test.ts @@ -1,7 +1,15 @@ import { describe, expect, it } from 'vitest'; -import { clampFloatingPosition, positionFloatingSubmenu } from './floatingPosition'; +import { clampFloatingPosition, isFloatingSubmenuCloseKey, positionFloatingSubmenu } from './floatingPosition'; describe('floating surface positioning', () => { + it('uses the physical back arrow for submenu dismissal in each direction', () => { + expect(isFloatingSubmenuCloseKey('ArrowLeft', false)).toBe(true); + expect(isFloatingSubmenuCloseKey('ArrowRight', false)).toBe(false); + expect(isFloatingSubmenuCloseKey('ArrowRight', true)).toBe(true); + expect(isFloatingSubmenuCloseKey('ArrowLeft', true)).toBe(false); + expect(isFloatingSubmenuCloseKey('Escape', true)).toBe(true); + }); + it('keeps a variable-height menu inside the viewport', () => { expect(clampFloatingPosition(350, 580, 192, 220, 400, 640)).toEqual({ x: 200, diff --git a/src/utils/floatingPosition.ts b/src/utils/floatingPosition.ts index 62d97a3..ccfeeea 100644 --- a/src/utils/floatingPosition.ts +++ b/src/utils/floatingPosition.ts @@ -5,6 +5,9 @@ export interface FloatingPosition { export type FloatingSubmenuSide = 'left' | 'right'; +export const isFloatingSubmenuCloseKey = (key: string, isRtl: boolean): boolean => + key === 'Escape' || key === (isRtl ? 'ArrowRight' : 'ArrowLeft'); + export interface FloatingSubmenuPosition extends FloatingPosition { side: FloatingSubmenuSide; }