Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 28, 2025

Dropping a piece onto a valid empty spot immediately re-selected that point due to both onDrop and onPointerUp handlers firing before React state updates propagated.

Changes

  • Added dropOccurredRef to track drop events synchronously in Point.tsx
  • Modified onDrop to set flag, preventing onPointerUp from executing selection logic
  • Flag resets via setTimeout(..., 0) after event loop completes
const onDrop: DragEventHandler = useCallback((event) => {
    event.preventDefault();
    setDragOver(false);
    dropOccurredRef.current = true;
    setTimeout(() => {
        dropOccurredRef.current = false;
    }, 0);
    const from = parseDragData(event.dataTransfer?.getData("text"))
    return move(from, position)
}, [move, position])

const onPointerUp = useCallback(() => {
    if (dragging || dropOccurredRef.current) return;
    // ... rest of selection logic
}, [position, onSelect, dragging, enabled, selected, move, valid])

Uses ref instead of state for immediate synchronous access, avoiding React's async update delay.

Original prompt

if i drop a piece onto a valid empty spot, it is reselecting the point immediately after dropping. i think this is a race condition bug


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: ProLoser <67395+ProLoser@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix race condition bug after dropping a piece Fix race condition causing point re-selection after drag-and-drop Dec 28, 2025
Copilot AI requested a review from ProLoser December 28, 2025 07:30
@ProLoser
Copy link
Owner

@copilot couldn't you use event.stopPropagation() instead

Copy link
Contributor Author

Copilot AI commented Dec 28, 2025

@copilot couldn't you use event.stopPropagation() instead

event.stopPropagation() wouldn't work here because onDrop and onPointerUp are different event types (drop vs pointerup). The browser fires both events sequentially during a drag-and-drop operation, and stopping propagation on one event type doesn't prevent a different event type from firing.

The dropOccurredRef approach is needed to coordinate between these two different event handlers.

@ProLoser
Copy link
Owner

ProLoser commented Jan 5, 2026

@copilot sounds like dragging is unreliable

Copy link
Contributor Author

Copilot AI commented Jan 5, 2026

@copilot sounds like dragging is unreliable

The dragging state alone isn't sufficient because of event ordering: when a drop occurs, onDragEnd fires before onPointerUp, setting dragging back to false. This means onPointerUp sees dragging=false and proceeds with the selection logic.

The dropOccurredRef flag bridges this gap by staying true just long enough to prevent onPointerUp from executing after a successful drop completes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants