Skip to content

fix: Esc restores item dragged out of its grid - #3443

Open
SinnerAir wants to merge 2 commits into
gridstack:masterfrom
SinnerAir:fix/cancel-drag-after-leave
Open

SinnerAir wants to merge 2 commits into
gridstack:masterfrom
SinnerAir:fix/cancel-drag-after-leave

Conversation

@SinnerAir

Copy link
Copy Markdown

Description

Pressing Esc doesn't revert a drag once the item has left its own grid (dragged past the grid edge, or over a grid that doesn't accept it): the item stays at its last position inside the grid and the nodes it pushed stay moved, and a change event is sent.

Steps (any grid with acceptWidgets: true): drag a over its neighbour b so b gets pushed, keep dragging out of the grid, press Esc — a lands on b's old spot and b stays pushed.

Cause: _leave() removes the node from the engine (_temporaryRemoved), so cancelDrag() -> engine.restoreInitial() restores only the other nodes. onEndMoving() then re-adds the node at its last tried position (not _orig), pushing the just restored nodes again.

Fix: in cancelDrag(), a node of ours that was temporarily removed is reset back to its _orig position, which onEndMoving() adds it back at.

Related but not fixed here: #2933 (Esc while over another grid that accepts the item).

Checklist

  • Created tests which fail without the change (if possible) — e2e/cancelled-drag-after-leave.spec.ts (float: false and true), fails on master
  • All tests passing (yarn test) — vitest 446/446, tsc + eslint src; the 6 failures in e2e/gridstack-e2e.spec.ts (getGrids is not a function) are the same on master
  • Extended the README / documentation, if necessary — n/a

When an item is dragged out of its own grid (past the edge, or over a grid
that doesn't accept it), _leave() removes it from the engine, so
cancelDrag() -> engine.restoreInitial() only restores the other nodes.
onEndMoving() then re-adds the item at its last position (not _orig),
pushing the just restored nodes again.

cancelDrag() now resets such an item back to its _orig position, which
onEndMoving() adds it back at.

This branch has not been deployed

No deployments
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.

1 participant