Merge pull request #3422 from adumesny/master

* fix #1959 refuse a move that would land on a locked item

Author: adumesny

doc/CHANGES.md

@@ -149,6 +149,7 @@ Change log
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
 ## 14.0.1 (TBD)
+* fix: [#1959](https://github.com/gridstack/gridstack.js/issues/1959) `update()` refuses a move that would land on a locked item
 * fix: [#2666](https://github.com/gridstack/gridstack.js/issues/2666) iOS auto-scroll uses visualViewport, and un-bind touch handlers
 * 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)
 * 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

@@ -1,5 +1,5 @@
 import { GridItemHTMLElement, GridStack, GridStackWidget } from '../src/gridstack';
-import type { GridStackNode } from '../src/types';
+import type { GridStackNode, GridStackMode } from '../src/types';
 import { Utils } from '../src/utils';
 import { DDElement } from '../src/dd-element';
 import { DDDraggable } from '../src/dd-draggable';
@@ -819,4 +819,69 @@ describe('regression >', () => {
       expect(ch).toBeGreaterThan(0);
     });
   });
+
+  describe('1959 update() must not overlap a locked item >', () => {
+    beforeEach(() => {
+      document.body.insertAdjacentHTML('afterbegin', gridstackEmptyHTML);
+    });
+    afterEach(() => {
+      document.body.removeChild(document.getElementById('gs-cont'));
+    });
+
+    const overlaps = (a: GridStackNode, b: GridStackNode): boolean =>
+      a.y! < b.y! + b.h! && b.y! < a.y! + a.h! && a.x! < b.x! + b.w! && b.x! < a.x! + a.w!;
+
+    const setup = (mode: GridStackMode, lockY: number, startY: number) => {
+      grid?.destroy(false);
+      document.getElementById('gs-cont')!.innerHTML = '<div class="grid-stack"></div>';
+      grid = GridStack.init({column: 12, cellHeight: 50, mode, children: [
+        {id: 'lock', x: 0, y: lockY, w: 2, h: 2, locked: true, noMove: true, noResize: true},
+        {id: 'move', x: 0, y: startY, w: 2, h: 2},
+      ]});
+      const move = grid.engine.nodes.find(n => n.id === 'move')!;
+      const lock = grid.engine.nodes.find(n => n.id === 'lock')!;
+      expect(overlaps(move, lock)).toBe(false); // sane starting layout
+      return { move, lock };
+    };
+
+    it('stays put rather than overlapping, across modes and targets', () => {
+      const bad: string[] = [];
+      (['top', 'float', 'list', 'compact'] as GridStackMode[]).forEach(mode => {
+        [0, 2, 4].forEach(lockY => {
+          [6, 7].forEach(startY => {
+            [0, 1, 2, 3, 4, 5].forEach(targetY => {
+              const { move, lock } = setup(mode, lockY, startY);
+              grid.update(move.el!, {y: targetY});
+              if (overlaps(move, lock)) {
+                bad.push(`${mode} lock@${lockY} ${startY}->${targetY} landed @${move.y}`);
+              }
+            });
+          });
+        });
+      });
+      expect(bad).toEqual([]);
+    });
+
+    it('still moves when the locked item is not in the way', () => {
+      const { move, lock } = setup('top', 0, 6);
+      grid.update(move.el!, {y: 2}); // right below the locked rows 0-1
+      expect(move.y).toBe(2);
+      expect(overlaps(move, lock)).toBe(false);
+    });
+
+    it('still pushes a plain (unlocked) neighbour out of the way', () => {
+      grid?.destroy(false);
+      document.getElementById('gs-cont')!.innerHTML = '<div class="grid-stack"></div>';
+      grid = GridStack.init({column: 12, cellHeight: 50, mode: 'float', children: [
+        {id: 'free', x: 0, y: 0, w: 2, h: 2},
+        {id: 'move', x: 0, y: 6, w: 2, h: 2},
+      ]});
+      const move = grid.engine.nodes.find(n => n.id === 'move')!;
+      const free = grid.engine.nodes.find(n => n.id === 'free')!;
+      grid.update(move.el!, {y: 0});
+      expect(overlaps(move, free)).toBe(false);
+      expect(move.y).toBe(0); // took the spot, pushed the other one
+      expect(free.y).toBe(2);
+    });
+  });
 });

src/gridstack-engine.ts

@@ -1066,6 +1066,12 @@ export class GridStackEngine {
         }
       }
 
+      // never land on top of a LOCKED item
+      if (needToMove && this.collideAll(node, nn, o.skip).some(n => n.locked)) {
+        needToMove = false;
+        if (wasUndefinedPack) delete o.pack;
+      }
+
       // now move (to the original ask vs the collision version which might differ) and repack things
       if (needToMove && !Utils.samePos(node, nn)) {
         node._dirty = true;