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

api.tryReleaseLock does not abort an existing drag #1699

Closed
nerdkid93 opened this issue Jan 7, 2020 · 3 comments · Fixed by #1702
Closed

api.tryReleaseLock does not abort an existing drag #1699

nerdkid93 opened this issue Jan 7, 2020 · 3 comments · Fixed by #1702

Comments

@nerdkid93
Copy link

Expected behavior

I'd expect useKeyboardSensor to reset the lock state when it aborts before it can call api.drop() on a Droppable, or phrased another way, I'd expect to be able to tryGetLock after having successfully called tryReleaseLock on the sensorApi.

Actual behavior

useKeyboardSensor only cleans up it's event handlers but doesn't reset the state of the DragDropContext so nothing can get drag-and-dropped until you either refresh or force re-render the parent component.

Steps to reproduce

Make a <DragDropContext sensors={[useNoKeyboardSensors]} /> where useNoKeyboardSensors is the following hook:

function useNoKeyboardSensors( api ) {
	const abortKeyboardSensor = useCallback( ( e ) => {
		if ( ![" ", "Spacebar" ].includes( e.key ) ) {
			return;
		}

		const draggableId = api.findClosestDraggableId( e );
		if ( !draggableId ) {
			return;
		}

		const canLock = api.canGetLock( draggableId );
		if ( !canLock ) {
			api.tryReleaseLock();
		}

		const preDrag = api.tryGetLock( draggableId );
		if ( preDrag !== null ) {
			// unreachable code as of `[email protected]`
			preDrag.abort();
		}
	}, [api]);

	useEffect(() => {
		window.addEventListener( 'keydown', abortKeyboardSensor );
		return () => window.removeEventListener( 'keydown', abortKeyboardSensor );
	}, [abortKeyboardSensor]);
}

Suggested solution?

I'm not sure I have any ideas for a good solution to this problem 😞

What version of React are you using?

16.11.0

What version of react-beautiful-dnd are you running?

12.2.0

What browser are you using?

Both Chrome Dev and Chrome Stable (v79 and v80)

Demo

https://codesandbox.io/s/vertical-list-c6fod

@alexreardon
Copy link
Collaborator

I think we will fix your issue by allowing you to disable the keyboard sensor. But i think it is also worth looking into the issue you are experiencing too

@alexreardon
Copy link
Collaborator

I'm struggling to wrap my head around this:

I'd expect useKeyboardSensor to reset the lock state when it aborts before it can call api.drop() on a Droppable, or phrased another way, I'd expect to be able to tryGetLock after having successfully called tryReleaseLock on the sensorApi.

Can you please phrase it another way?

@alexreardon
Copy link
Collaborator

I can see the issue.

If a lock is forcefully aborted using api.tryReleaseLock and there is an active drag; that drag is not aborted (just the sensor lock is releases)

I should have a fix up soon

@alexreardon alexreardon changed the title useKeyboardSensor does not surrender sensorApi lock when released/aborted api.tryReleaseLock does not abort an existing drag Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@alexreardon @nerdkid93 and others