Skip to content

Commit d46f09c

Browse files
committed
* fix #1959 refuse a move that would land on a locked item
1 parent f33b895 commit d46f09c

3 files changed

Lines changed: 73 additions & 1 deletion

File tree

‎doc/CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ Change log
149149
<!-- END doctoc generated TOC please keep comment here to allow auto update -->
150150

151151
## 14.0.1 (TBD)
152+
* fix: [#1959](https://github.com/gridstack/gridstack.js/issues/1959) `update()` refuses a move that would land on a locked item
152153
* fix: [#2666](https://github.com/gridstack/gridstack.js/issues/2666) iOS auto-scroll uses visualViewport, and un-bind touch handlers
153154
* fix: [#3418](https://github.com/gridstack/gridstack.js/pull/3418) preserve saved layout after cancelling a cross-grid drag- #3418 - thank you [sameerdeolalikar](https://github.com/sameerdeolalikar)
154155
* fix: [#2819](https://github.com/gridstack/gridstack.js/issues/2819) item not moved (to alt position) when colliding fails with an object

‎spec/regression-spec.ts‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { GridItemHTMLElement, GridStack, GridStackWidget } from '../src/gridstack';
2-
import type { GridStackNode } from '../src/types';
2+
import type { GridStackNode, GridStackMode } from '../src/types';
33
import { Utils } from '../src/utils';
44
import { DDElement } from '../src/dd-element';
55
import { DDDraggable } from '../src/dd-draggable';
@@ -819,4 +819,69 @@ describe('regression >', () => {
819819
expect(ch).toBeGreaterThan(0);
820820
});
821821
});
822+
823+
describe('1959 update() must not overlap a locked item >', () => {
824+
beforeEach(() => {
825+
document.body.insertAdjacentHTML('afterbegin', gridstackEmptyHTML);
826+
});
827+
afterEach(() => {
828+
document.body.removeChild(document.getElementById('gs-cont'));
829+
});
830+
831+
const overlaps = (a: GridStackNode, b: GridStackNode): boolean =>
832+
a.y! < b.y! + b.h! && b.y! < a.y! + a.h! && a.x! < b.x! + b.w! && b.x! < a.x! + a.w!;
833+
834+
const setup = (mode: GridStackMode, lockY: number, startY: number) => {
835+
grid?.destroy(false);
836+
document.getElementById('gs-cont')!.innerHTML = '<div class="grid-stack"></div>';
837+
grid = GridStack.init({column: 12, cellHeight: 50, mode, children: [
838+
{id: 'lock', x: 0, y: lockY, w: 2, h: 2, locked: true, noMove: true, noResize: true},
839+
{id: 'move', x: 0, y: startY, w: 2, h: 2},
840+
]});
841+
const move = grid.engine.nodes.find(n => n.id === 'move')!;
842+
const lock = grid.engine.nodes.find(n => n.id === 'lock')!;
843+
expect(overlaps(move, lock)).toBe(false); // sane starting layout
844+
return { move, lock };
845+
};
846+
847+
it('stays put rather than overlapping, across modes and targets', () => {
848+
const bad: string[] = [];
849+
(['top', 'float', 'list', 'compact'] as GridStackMode[]).forEach(mode => {
850+
[0, 2, 4].forEach(lockY => {
851+
[6, 7].forEach(startY => {
852+
[0, 1, 2, 3, 4, 5].forEach(targetY => {
853+
const { move, lock } = setup(mode, lockY, startY);
854+
grid.update(move.el!, {y: targetY});
855+
if (overlaps(move, lock)) {
856+
bad.push(`${mode} lock@${lockY} ${startY}->${targetY} landed @${move.y}`);
857+
}
858+
});
859+
});
860+
});
861+
});
862+
expect(bad).toEqual([]);
863+
});
864+
865+
it('still moves when the locked item is not in the way', () => {
866+
const { move, lock } = setup('top', 0, 6);
867+
grid.update(move.el!, {y: 2}); // right below the locked rows 0-1
868+
expect(move.y).toBe(2);
869+
expect(overlaps(move, lock)).toBe(false);
870+
});
871+
872+
it('still pushes a plain (unlocked) neighbour out of the way', () => {
873+
grid?.destroy(false);
874+
document.getElementById('gs-cont')!.innerHTML = '<div class="grid-stack"></div>';
875+
grid = GridStack.init({column: 12, cellHeight: 50, mode: 'float', children: [
876+
{id: 'free', x: 0, y: 0, w: 2, h: 2},
877+
{id: 'move', x: 0, y: 6, w: 2, h: 2},
878+
]});
879+
const move = grid.engine.nodes.find(n => n.id === 'move')!;
880+
const free = grid.engine.nodes.find(n => n.id === 'free')!;
881+
grid.update(move.el!, {y: 0});
882+
expect(overlaps(move, free)).toBe(false);
883+
expect(move.y).toBe(0); // took the spot, pushed the other one
884+
expect(free.y).toBe(2);
885+
});
886+
});
822887
});

‎src/gridstack-engine.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,12 @@ export class GridStackEngine {
10661066
}
10671067
}
10681068

1069+
// never land on top of a LOCKED item
1070+
if (needToMove && this.collideAll(node, nn, o.skip).some(n => n.locked)) {
1071+
needToMove = false;
1072+
if (wasUndefinedPack) delete o.pack;
1073+
}
1074+
10691075
// now move (to the original ask vs the collision version which might differ) and repack things
10701076
if (needToMove && !Utils.samePos(node, nn)) {
10711077
node._dirty = true;

0 commit comments

Comments
 (0)