Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix react18 concurrent mode drag glitch due to state inconsistency #699

Merged
merged 2 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 25 additions & 38 deletions lib/DraggableCore.js
Expand Up @@ -28,13 +28,6 @@ const eventsFor = {
// Default to mouse events.
let dragEventFor = eventsFor.mouse;

type DraggableCoreState = {
dragging: boolean,
lastX: number,
lastY: number,
touchIdentifier: ?number
};

export type DraggableData = {
node: HTMLElement,
x: number, y: number,
Expand Down Expand Up @@ -75,7 +68,7 @@ export type DraggableCoreProps = {
// work well with libraries that require more control over the element.
//

export default class DraggableCore extends React.Component<DraggableCoreProps, DraggableCoreState> {
export default class DraggableCore extends React.Component<DraggableCoreProps> {

static displayName: ?string = 'DraggableCore';

Expand Down Expand Up @@ -227,12 +220,13 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
scale: 1,
};

state: DraggableCoreState = {
dragging: false,
// Used while dragging to determine deltas.
lastX: NaN, lastY: NaN,
touchIdentifier: null
};
dragging: boolean = false;

// Used while dragging to determine deltas.
lastX: number = NaN;
lastY: number = NaN;

touchIdentifier: ?number = null;

mounted: boolean = false;

Expand Down Expand Up @@ -298,7 +292,7 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
// distinguish between individual touches on multitouch screens by identifying which
// touchpoint was set to this element.
const touchIdentifier = getTouchIdentifier(e);
this.setState({touchIdentifier});
this.touchIdentifier = touchIdentifier;

// Get the current drag point from the event. This is used as the offset.
const position = getControlPosition(e, touchIdentifier, this);
Expand All @@ -322,12 +316,9 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
// Initiate dragging. Set the current x and y as offsets
// so we know how much we've moved during the drag. This allows us
// to drag elements around even if they have been moved, without issue.
this.setState({
dragging: true,

lastX: x,
lastY: y
});
this.dragging = true;
this.lastX = x;
this.lastY = y;

// Add events to the document directly so we catch when the user's mouse/touch moves outside of
// this element. We use different events depending on whether or not we have detected that this
Expand All @@ -339,16 +330,16 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
handleDrag: EventHandler<MouseTouchEvent> = (e) => {

// Get the current drag point from the event. This is used as the offset.
const position = getControlPosition(e, this.state.touchIdentifier, this);
const position = getControlPosition(e, this.touchIdentifier, this);
if (position == null) return;
let {x, y} = position;

// Snap to grid if prop has been provided
if (Array.isArray(this.props.grid)) {
let deltaX = x - this.state.lastX, deltaY = y - this.state.lastY;
let deltaX = x - this.lastX, deltaY = y - this.lastY;
[deltaX, deltaY] = snapToGrid(this.props.grid, deltaX, deltaY);
if (!deltaX && !deltaY) return; // skip useless drag
x = this.state.lastX + deltaX, y = this.state.lastY + deltaY;
x = this.lastX + deltaX, y = this.lastY + deltaY;
}

const coreEvent = createCoreData(this, x, y);
Expand All @@ -372,25 +363,23 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
return;
}

this.setState({
lastX: x,
lastY: y
});
this.lastX = x;
this.lastY = y;
};

handleDragStop: EventHandler<MouseTouchEvent> = (e) => {
if (!this.state.dragging) return;
if (!this.dragging) return;

const position = getControlPosition(e, this.state.touchIdentifier, this);
const position = getControlPosition(e, this.touchIdentifier, this);
if (position == null) return;
let {x, y} = position;

// Snap to grid if prop has been provided
if (Array.isArray(this.props.grid)) {
let deltaX = x - this.state.lastX || 0;
let deltaY = y - this.state.lastY || 0;
let deltaX = x - this.lastX || 0;
let deltaY = y - this.lastY || 0;
[deltaX, deltaY] = snapToGrid(this.props.grid, deltaX, deltaY);
x = this.state.lastX + deltaX, y = this.state.lastY + deltaY;
x = this.lastX + deltaX, y = this.lastY + deltaY;
}

const coreEvent = createCoreData(this, x, y);
Expand All @@ -408,11 +397,9 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
log('DraggableCore: handleDragStop: %j', coreEvent);

// Reset the el.
this.setState({
dragging: false,
lastX: NaN,
lastY: NaN
});
this.dragging = false;
this.lastX = NaN;
this.lastY = NaN;

if (thisNode) {
// Remove event handlers
Expand Down
7 changes: 3 additions & 4 deletions lib/utils/positionFns.js
Expand Up @@ -78,8 +78,7 @@ export function getControlPosition(e: MouseTouchEvent, touchIdentifier: ?number,

// Create an data object exposed by <DraggableCore>'s events
export function createCoreData(draggable: DraggableCore, x: number, y: number): DraggableData {
const state = draggable.state;
const isStart = !isNum(state.lastX);
const isStart = !isNum(draggable.lastX);
const node = findDOMNode(draggable);

if (isStart) {
Expand All @@ -94,8 +93,8 @@ export function createCoreData(draggable: DraggableCore, x: number, y: number):
// Otherwise calculate proper values.
return {
node,
deltaX: x - state.lastX, deltaY: y - state.lastY,
lastX: state.lastX, lastY: state.lastY,
deltaX: x - draggable.lastX, deltaY: y - draggable.lastY,
lastX: draggable.lastX, lastY: draggable.lastY,
x, y,
};
}
Expand Down