fix: Address Copilot review feedback for performance optimizations

Fix issues identified in Copilot review:

1. Remove throttling from stage drag cursor updates - cursor now updates immediately on drag start for better UX
2. Accumulate wheel deltaY values during throttle period instead of only processing last event - prevents jerky zoom behavior
3. Remove redundant listening={false} props from child elements (inherited from parent Group)
4. Update file documentation to reflect stage drag cursor update functionality

These changes improve both performance and user experience while maintaining code clarity.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Henrik Bruhn 2025-12-28 14:08:08 +01:00
parent 93ebc8398c
commit 9d30eae901
2 changed files with 31 additions and 50 deletions

View file

@ -53,7 +53,6 @@ export const Grid = memo(({ gridSize, bounds, machineInfo }: GridProps) => {
points={points}
stroke={gridColor}
strokeWidth={1}
listening={false}
/>
))}
{lines.horizontalLines.map((points, i) => (
@ -62,7 +61,6 @@ export const Grid = memo(({ gridSize, bounds, machineInfo }: GridProps) => {
points={points}
stroke={gridColor}
strokeWidth={1}
listening={false}
/>
))}
</Group>
@ -76,18 +74,8 @@ export const Origin = memo(() => {
return (
<Group name="origin" listening={false}>
<Line
points={[-10, 0, 10, 0]}
stroke={originColor}
strokeWidth={2}
listening={false}
/>
<Line
points={[0, -10, 0, 10]}
stroke={originColor}
strokeWidth={2}
listening={false}
/>
<Line points={[-10, 0, 10, 0]} stroke={originColor} strokeWidth={2} />
<Line points={[0, -10, 0, 10]} stroke={originColor} strokeWidth={2} />
</Group>
);
});
@ -114,7 +102,6 @@ export const Hoop = memo(({ machineInfo }: HoopProps) => {
stroke={hoopColor}
strokeWidth={3}
dash={[10, 5]}
listening={false}
/>
<Text
x={hoopLeft + 10}
@ -124,7 +111,6 @@ export const Hoop = memo(({ machineInfo }: HoopProps) => {
fontFamily="sans-serif"
fontStyle="bold"
fill={hoopColor}
listening={false}
/>
</Group>
);

View file

@ -2,7 +2,7 @@
* useCanvasViewport Hook
*
* Manages canvas viewport state including zoom, pan, and container size
* Handles wheel zoom and button zoom operations
* Handles wheel zoom, button zoom operations, and stage drag cursor updates
*/
import {
@ -93,43 +93,45 @@ export function useCanvasViewport({
setStagePos({ x: containerSize.width / 2, y: containerSize.height / 2 });
}
// Wheel zoom handler with RAF throttling
// Wheel zoom handler with RAF throttling and delta accumulation
const wheelThrottleRef = useRef<number | null>(null);
const wheelEventRef = useRef<Konva.KonvaEventObject<WheelEvent> | null>(null);
const accumulatedDeltaRef = useRef<number>(0);
const lastPointerRef = useRef<{ x: number; y: number } | null>(null);
const lastStageRef = useRef<Konva.Stage | null>(null);
const handleWheel = useCallback((e: Konva.KonvaEventObject<WheelEvent>) => {
e.evt.preventDefault();
// Store the latest event
wheelEventRef.current = e;
const stage = e.target.getStage();
if (!stage) return;
// Cancel pending throttle if it exists
const pointer = stage.getPointerPosition();
if (!pointer) return;
// Accumulate deltaY from all events during throttle period
accumulatedDeltaRef.current += e.evt.deltaY;
lastPointerRef.current = pointer;
lastStageRef.current = stage;
// Skip if throttle already in progress
if (wheelThrottleRef.current !== null) {
return; // Throttle in progress, skip this event
return;
}
// Schedule update on next animation frame (~16ms)
wheelThrottleRef.current = requestAnimationFrame(() => {
const throttledEvent = wheelEventRef.current;
if (!throttledEvent) {
wheelThrottleRef.current = null;
return;
}
const accumulatedDelta = accumulatedDeltaRef.current;
const pointer = lastPointerRef.current;
const stage = lastStageRef.current;
const stage = throttledEvent.target.getStage();
if (!stage) {
wheelThrottleRef.current = null;
return;
}
const pointer = stage.getPointerPosition();
if (!pointer) {
if (!pointer || !stage || accumulatedDelta === 0) {
wheelThrottleRef.current = null;
accumulatedDeltaRef.current = 0;
return;
}
const scaleBy = 1.1;
const direction = throttledEvent.evt.deltaY > 0 ? -1 : 1;
const direction = accumulatedDelta > 0 ? -1 : 1;
setStageScale((oldScale) => {
const newScale = Math.max(
@ -145,8 +147,9 @@ export function useCanvasViewport({
return newScale;
});
// Reset accumulator and throttle
wheelThrottleRef.current = null;
wheelEventRef.current = null;
accumulatedDeltaRef.current = 0;
});
}, []);
@ -199,20 +202,13 @@ export function useCanvasViewport({
setStagePos({ x: containerSize.width / 2, y: containerSize.height / 2 });
}, [initialScale, containerSize]);
// Stage drag handlers with throttled cursor updates
const lastCursorUpdateRef = useRef<number>(0);
// Stage drag handlers - cursor updates immediately for better UX
const handleStageDragStart = useCallback(
(e: Konva.KonvaEventObject<DragEvent>) => {
const now = Date.now();
// Throttle cursor updates to ~60fps (16ms)
if (now - lastCursorUpdateRef.current > 16) {
const stage = e.target.getStage();
if (stage) {
stage.container().style.cursor = "grabbing";
}
lastCursorUpdateRef.current = now;
}
},
[],
);
@ -223,7 +219,6 @@ export function useCanvasViewport({
if (stage) {
stage.container().style.cursor = "grab";
}
lastCursorUpdateRef.current = 0;
},
[],
);