Skip to content

fix for locked widgets still moving by others #1219

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.

Sign up for GitHub

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

Merged
adumesny merged 1 commit into from
Mar 16, 2020
Merged

fix for locked widgets still moving by others #1219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions demo/experiment/locked.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Locked demo</title>

<link rel="stylesheet" href="../demo.css"/>
<script src="../../src/jquery.js"></script>
<script src="../../src/gridstack.js"></script>
<script src="../../src/jquery-ui.js"></script>
<script src="../../src/gridstack.jQueryUI.js"></script>
</style>
</head>
<body>
<div class="container-fluid">
<h1>Locked demo</h1>
<div>
<a class="btn btn-primary" onClick="addNewWidget()" href="#">Add Widget</a>
<a class="btn btn-primary" onclick="toggleFloat()" id="float" href="#">float: true</a>
</div>
<br><br>
<div class="grid-stack"></div>
</div>

<script type="text/javascript">
var grid = GridStack.init({float: true});

grid.on('added removed change', function(e, items) {
var str = '';
items.forEach(function(item) { str += ' (x,y)=' + item.x + ',' + item.y; });
console.log(e.type + ' ' + items.length + ' items:' + str );
});

var items = [
{x: 0, y: 1, width: 12, height: 1, locked: 'yes', noMove: true, noResize: true, text: 'locked, noResize, noMove'},
{x: 1, y: 0, width: 2, height: 3},
{x: 4, y: 2, width: 1, height: 1},
{x: 3, y: 1, width: 1, height: 2},
{x: 0, y: 6, width: 2, height: 2}
];
var count = 0;

addNewWidget = function() {
var n = items[count] || {
x: Math.round(12 * Math.random()),
y: Math.round(5 * Math.random()),
width: Math.round(1 + 3 * Math.random()),
height: Math.round(1 + 3 * Math.random())
};
grid.addWidget('<div><div class="grid-stack-item-content">' + (n.text ? n.text : count) + '</div></div>', n);
count++
};

toggleFloat = function() {
grid.float(! grid.float());
document.querySelector('#float').innerHTML = 'float: ' + grid.float();
};
addNewWidget();
</script>
</body>
</html>
2 changes: 2 additions & 0 deletions doc/CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ Change log

- fix [1187](https://github.com/gridstack/gridstack.js/issues/1187) IE support for `CustomEvent` polyfill - thanks [@phil-blais](https://github.com/phil-blais)
- fix [1204](https://github.com/gridstack/gridstack.js/issues/1204) destroy drag&drop when removing node(s) instead of just disabling it.
- fix [1181](https://github.com/gridstack/gridstack.js/issues/1181) Locked widgets are still moveable by other widgets.
- fix [1217](https://github.com/gridstack/gridstack.js/issues/1217) If I set cellHeight to some vh, only first grid will take vh, rest will use px
- include SASS source files to npm package again [1193](https://github.com/gridstack/gridstack.js/pull/1193)

## 1.1.0 (2020-02-29)
Expand Down
106 changes: 45 additions & 61 deletions spec/gridstack-engine-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
describe('gridstack engine', function() {
'use strict';

var e;
var w;
let engine;
let findNode = function(engine, id) {
return engine.nodes.find(function(i) { return i._id === id; });
};

beforeEach(function() {
w = window;
e = GridStack.Engine;
});

describe('test constructor', function() {
var engine;


beforeAll(function() {
engine = new GridStack.Engine(12);
});

it('should be setup properly', function() {
expect(engine.column).toEqual(12);
expect(engine.float).toEqual(false);
Expand All @@ -25,12 +22,10 @@ describe('gridstack engine', function() {
});

describe('test _prepareNode', function() {
var engine;

beforeAll(function() {
engine = new GridStack.Engine(12);
});

it('should prepare a node', function() {
expect(engine._prepareNode({}, false)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1}));
expect(engine._prepareNode({x: 10}, false)).toEqual(jasmine.objectContaining({x: 10, y: 0, width: 1, height: 1}));
Expand All @@ -50,7 +45,6 @@ describe('gridstack engine', function() {
});

describe('test isAreaEmpty', function() {
var engine;

beforeAll(function() {
engine = new GridStack.Engine(12, null, true);
Expand All @@ -71,7 +65,6 @@ describe('gridstack engine', function() {
});

describe('test cleanNodes/getDirtyNodes', function() {
var engine;

beforeAll(function() {
engine = new GridStack.Engine(12, null, true);
Expand All @@ -87,8 +80,7 @@ describe('gridstack engine', function() {
});

it('should return all dirty nodes', function() {
var nodes = engine.getDirtyNodes();

let nodes = engine.getDirtyNodes();
expect(nodes.length).toEqual(2);
expect(nodes[0].idx).toEqual(1);
expect(nodes[1].idx).toEqual(2);
Expand All @@ -97,20 +89,16 @@ describe('gridstack engine', function() {
it('should\'n clean nodes if _batchMode true', function() {
engine._batchMode = true;
engine.cleanNodes();

expect(engine.getDirtyNodes().length).toBeGreaterThan(0);
});

it('should clean all dirty nodes', function() {
engine.cleanNodes();

expect(engine.getDirtyNodes().length).toEqual(0);
});
});

describe('test batchUpdate/commit', function() {
var engine;

beforeAll(function() {
engine = new GridStack.Engine(12);
});
Expand All @@ -137,7 +125,6 @@ describe('gridstack engine', function() {
});

describe('test batchUpdate/commit', function() {
var engine;

beforeAll(function() {
engine = new GridStack.Engine(12, null, true);
Expand All @@ -155,17 +142,14 @@ describe('gridstack engine', function() {
});

describe('test _notify', function() {
var engine;
var spy;
let spy;

beforeEach(function() {
spy = {
callback: function() {}
};
spyOn(spy, 'callback');

engine = new GridStack.Engine(12, spy.callback, true);

engine.nodes = [
engine._prepareNode({x: 0, y: 0, width: 1, height: 1, idx: 1, _dirty: true}),
engine._prepareNode({x: 3, y: 2, width: 3, height: 2, idx: 2, _dirty: true}),
Expand All @@ -176,24 +160,20 @@ describe('gridstack engine', function() {
it('should\'n be called if _batchMode true', function() {
engine._batchMode = true;
engine._notify();

expect(spy.callback).not.toHaveBeenCalled();
});

it('should by called with dirty nodes', function() {
engine._notify();

expect(spy.callback).toHaveBeenCalledWith([
engine.nodes[0],
engine.nodes[1]
], true);
});

it('should by called with extra passed node to be removed', function() {
var n1 = {idx: -1};

let n1 = {idx: -1};
engine._notify(n1);

expect(spy.callback).toHaveBeenCalledWith([
n1,
engine.nodes[0],
Expand All @@ -202,10 +182,8 @@ describe('gridstack engine', function() {
});

it('should by called with extra passed node to be removed and should maintain false parameter', function() {
var n1 = {idx: -1};

let n1 = {idx: -1};
engine._notify(n1, false);

expect(spy.callback).toHaveBeenCalledWith([
n1,
engine.nodes[0],
Expand All @@ -216,12 +194,6 @@ describe('gridstack engine', function() {

describe('test _packNodes', function() {
describe('using not float mode', function() {
var engine;

var findNode = function(engine, id) {
return engine.nodes.find(function(i) { return i._id === id; });
};

beforeEach(function() {
engine = new GridStack.Engine(12, null, false);
});
Expand All @@ -230,9 +202,7 @@ describe('gridstack engine', function() {
engine.nodes = [
{x: 0, y: 0, width: 1, height: 1, _id: 1},
];

engine._packNodes();

expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1}));
expect(findNode(engine, 1)._dirty).toBeFalsy();
});
Expand All @@ -241,9 +211,7 @@ describe('gridstack engine', function() {
engine.nodes = [
{x: 0, y: 1, width: 1, height: 1, _id: 1},
];

engine._packNodes();

expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1, _dirty: true}));
});

Expand All @@ -252,33 +220,27 @@ describe('gridstack engine', function() {
{x: 0, y: 1, width: 1, height: 1, _id: 1},
{x: 0, y: 5, width: 1, height: 1, _id: 2},
];

engine._packNodes();

expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1, _dirty: true}));
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 1, height: 1, _dirty: true}));
});

it('should pack nodes correctly', function() {
engine.nodes = [
{x: 0, y: 5, width: 1, height: 1, _id: 1},
{x: 0, y: 1, width: 1, height: 1, _id: 2},
];

engine._packNodes();

expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1, _dirty: true}));
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 1, height: 1, _dirty: true}));
});

it('should respect locked nodes', function() {
engine.nodes = [
{x: 0, y: 1, width: 1, height: 1, _id: 1, locked: true},
{x: 0, y: 5, width: 1, height: 1, _id: 2},
];

engine._packNodes();

expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 1, height: 1}));
expect(findNode(engine, 1)._dirty).toBeFalsy();
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 2, width: 1, height: 1, _dirty: true}));
Expand All @@ -287,35 +249,57 @@ describe('gridstack engine', function() {
});

describe('test isNodeChangedPosition', function() {
var engine;

beforeAll(function() {
engine = new GridStack.Engine(12);
});

it('should return true for changed x', function() {
var widget = { x: 1, y: 2, width: 3, height: 4 };
let widget = { x: 1, y: 2, width: 3, height: 4 };
expect(engine.isNodeChangedPosition(widget, 2, 2)).toEqual(true);
});

it('should return true for changed y', function() {
var widget = { x: 1, y: 2, width: 3, height: 4 };
let widget = { x: 1, y: 2, width: 3, height: 4 };
expect(engine.isNodeChangedPosition(widget, 1, 1)).toEqual(true);
});

it('should return true for changed width', function() {
var widget = { x: 1, y: 2, width: 3, height: 4 };
let widget = { x: 1, y: 2, width: 3, height: 4 };
expect(engine.isNodeChangedPosition(widget, 2, 2, 4, 4)).toEqual(true);
});

it('should return true for changed height', function() {
var widget = { x: 1, y: 2, width: 3, height: 4 };
let widget = { x: 1, y: 2, width: 3, height: 4 };
expect(engine.isNodeChangedPosition(widget, 1, 2, 3, 3)).toEqual(true);
});

it('should return false for unchanged position', function() {
var widget = { x: 1, y: 2, width: 3, height: 4 };
let widget = { x: 1, y: 2, width: 3, height: 4 };
expect(engine.isNodeChangedPosition(widget, 1, 2, 3, 4)).toEqual(false);
});
});

describe('test locked widget', function() {
beforeAll(function() {
engine = new GridStack.Engine(12);
});
it('should add widgets around locked one', function() {
let nodes = [
{x: 0, y: 1, width: 12, height: 1, locked: 'yes', noMove: true, noResize: true, _id: 1},
{x: 1, y: 0, width: 2, height: 3, _id: 2}
];
// add locked item
engine.addNode(nodes[0])
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 12, height: 1, locked: 'yes'}));
engine.addNode(nodes[1])
// add item that moves past locked one
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 12, height: 1, locked: 'yes'}));
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 1, y: 2}));
// prevents moving locked item
let node1 = findNode(engine, 1);
expect(engine.moveNode(node1, 6, 6)).toEqual(null);
// but moves regular one (gravity ON)
let node2 = findNode(engine, 2);
expect(engine.moveNode(node2, 6, 6)).toEqual(jasmine.objectContaining({x: 6, y: 2, width: 2, height: 3,}));
// but moves regular one (gravity OFF)
engine.float = true;
expect(engine.moveNode(node2, 7, 6)).toEqual(jasmine.objectContaining({x: 7, y: 6, width: 2, height: 3,}));
});
});

});
12 changes: 10 additions & 2 deletions src/gridstack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,15 @@
while (true) {
var collisionNode = this.nodes.find(Utils._collisionNodeCheck, {node: node, nn: nn});
if (!collisionNode) { return; }
var moved = this.moveNode(collisionNode, collisionNode.x, node.y + node.height,
collisionNode.width, collisionNode.height, true);
var moved;
if (collisionNode.locked) {
// if colliding with a locked item, move ourself instead
moved = this.moveNode(node, node.x, collisionNode.y + collisionNode.height,
node.width, node.height, true);
} else {
moved = this.moveNode(collisionNode, collisionNode.x, node.y + node.height,
collisionNode.width, collisionNode.height, true);
}
if (!moved) { return; } // break inf loop if we couldn't move after all (ex: maxRow, fixed)
}
};
Expand Down Expand Up @@ -635,6 +642,7 @@
};

GridStackEngine.prototype.moveNode = function(node, x, y, width, height, noPack) {
if (node.locked) { return null; }
if (typeof x !== 'number') { x = node.x; }
if (typeof y !== 'number') { y = node.y; }
if (typeof width !== 'number') { width = node.width; }
Expand Down