mirror of
https://github.com/nimbold/Firelink.git
synced 2026-09-02 14:07:57 +00:00
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
This commit is contained in:
@@ -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<DownloadTableProps> = ({ 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<DownloadTableProps> = ({ 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<DownloadTableProps> = ({ 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);
|
||||
|
||||
@@ -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<FloatingQueueSubmenuProps> = ({ labe
|
||||
}
|
||||
}}
|
||||
onKeyDown={event => {
|
||||
if (event.key !== 'Escape' && event.key !== 'ArrowLeft') return;
|
||||
if (!isFloatingSubmenuCloseKey(event.key, isRtl)) return;
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
closeMenu();
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { createColumnResizeSession } from './columnResize';
|
||||
|
||||
const createEventTarget = () => {
|
||||
const listeners = new Map<string, Set<EventListener>>();
|
||||
const target = {
|
||||
addEventListener: vi.fn((type: string, listener: EventListener) => {
|
||||
const current = listeners.get(type) ?? new Set<EventListener>();
|
||||
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<PointerEvent> = {}) => {
|
||||
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<string>();
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,61 @@
|
||||
type ResizeEventTarget = Pick<Window, 'addEventListener' | 'removeEventListener'>;
|
||||
type ResizeDocumentTarget = Pick<Document, 'addEventListener' | 'removeEventListener'>;
|
||||
type ResizeBody = Pick<HTMLElement, 'classList'>;
|
||||
|
||||
/** 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;
|
||||
};
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -45,8 +45,7 @@ export const countDownloadActions = (
|
||||
downloads: ReadonlyArray<{ status: DownloadStatus }>
|
||||
): DownloadActionCounts => downloads.reduce<DownloadActionCounts>((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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -3,6 +3,17 @@ export interface DownloadSelectionResult {
|
||||
lastSelectedId: string | null;
|
||||
}
|
||||
|
||||
export const selectContextMenuTarget = ({
|
||||
selectedIds,
|
||||
lastSelectedId,
|
||||
targetId,
|
||||
}: Pick<DownloadSelectionResult, 'selectedIds' | 'lastSelectedId'> & { 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<string>;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user