Skip to content

Commit c8e4c10

Browse files
adumesnyadumesny
committed
fix for locked widgets still moving by others
* make sure `_fixCollisions()` take node locked state into account * `moveNode()` also checks for locked state, but resize can still happen (forced for column layout change) * added demo and test case showing issue * fix #1181
1 parent 04a3412 commit c8e4c10

File tree

4 files changed

+120
-63
lines changed

4 files changed

+120
-63
lines changed

‎demo/experiment/locked.html‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<meta http-equiv="X-UA-Compatible" content="IE=edge">
6+
<meta name="viewport" content="width=device-width, initial-scale=1">
7+
<title>Locked demo</title>
8+
9+
<link rel="stylesheet" href="../demo.css"/>
10+
<script src="../../src/jquery.js"></script>
11+
<script src="../../src/gridstack.js"></script>
12+
<script src="../../src/jquery-ui.js"></script>
13+
<script src="../../src/gridstack.jQueryUI.js"></script>
14+
</style>
15+
</head>
16+
<body>
17+
<div class="container-fluid">
18+
<h1>Locked demo</h1>
19+
<div>
20+
<a class="btn btn-primary" onClick="addNewWidget()" href="#">Add Widget</a>
21+
<a class="btn btn-primary" onclick="toggleFloat()" id="float" href="#">float: true</a>
22+
</div>
23+
<br><br>
24+
<div class="grid-stack"></div>
25+
</div>
26+
27+
<script type="text/javascript">
28+
var grid = GridStack.init({float: true});
29+
30+
grid.on('added removed change', function(e, items) {
31+
var str = '';
32+
items.forEach(function(item) { str += ' (x,y)=' + item.x + ',' + item.y; });
33+
console.log(e.type + ' ' + items.length + ' items:' + str );
34+
});
35+
36+
var items = [
37+
{x: 0, y: 1, width: 12, height: 1, locked: 'yes', noMove: true, noResize: true, text: 'locked, noResize, noMove'},
38+
{x: 1, y: 0, width: 2, height: 3},
39+
{x: 4, y: 2, width: 1, height: 1},
40+
{x: 3, y: 1, width: 1, height: 2},
41+
{x: 0, y: 6, width: 2, height: 2}
42+
];
43+
var count = 0;
44+
45+
addNewWidget = function() {
46+
var n = items[count] || {
47+
x: Math.round(12 * Math.random()),
48+
y: Math.round(5 * Math.random()),
49+
width: Math.round(1 + 3 * Math.random()),
50+
height: Math.round(1 + 3 * Math.random())
51+
};
52+
grid.addWidget('<div><div class="grid-stack-item-content">' + (n.text ? n.text : count) + '</div></div>', n);
53+
count++
54+
};
55+
56+
toggleFloat = function() {
57+
grid.float(! grid.float());
58+
document.querySelector('#float').innerHTML = 'float: ' + grid.float();
59+
};
60+
addNewWidget();
61+
</script>
62+
</body>
63+
</html>

‎doc/CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ Change log
3636

3737
- fix [1187](https://github.com/gridstack/gridstack.js/issues/1187) IE support for `CustomEvent` polyfill - thanks [@phil-blais](https://github.com/phil-blais)
3838
- fix [1204](https://github.com/gridstack/gridstack.js/issues/1204) destroy drag&drop when removing node(s) instead of just disabling it.
39+
- fix [1181](https://github.com/gridstack/gridstack.js/issues/1181) Locked widgets are still moveable by other widgets.
40+
- 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
3941
- include SASS source files to npm package again [1193](https://github.com/gridstack/gridstack.js/pull/1193)
4042

4143
## 1.1.0 (2020-02-29)

‎spec/gridstack-engine-spec.js‎

Lines changed: 45 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,18 @@
11
describe('gridstack engine', function() {
22
'use strict';
3-
4-
var e;
5-
var w;
3+
let engine;
4+
let findNode = function(engine, id) {
5+
return engine.nodes.find(function(i) { return i._id === id; });
6+
};
67

78
beforeEach(function() {
8-
w = window;
9-
e = GridStack.Engine;
109
});
1110

1211
describe('test constructor', function() {
13-
var engine;
14-
12+
1513
beforeAll(function() {
1614
engine = new GridStack.Engine(12);
1715
});
18-
1916
it('should be setup properly', function() {
2017
expect(engine.column).toEqual(12);
2118
expect(engine.float).toEqual(false);
@@ -25,12 +22,10 @@ describe('gridstack engine', function() {
2522
});
2623

2724
describe('test _prepareNode', function() {
28-
var engine;
2925

3026
beforeAll(function() {
3127
engine = new GridStack.Engine(12);
3228
});
33-
3429
it('should prepare a node', function() {
3530
expect(engine._prepareNode({}, false)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1}));
3631
expect(engine._prepareNode({x: 10}, false)).toEqual(jasmine.objectContaining({x: 10, y: 0, width: 1, height: 1}));
@@ -50,7 +45,6 @@ describe('gridstack engine', function() {
5045
});
5146

5247
describe('test isAreaEmpty', function() {
53-
var engine;
5448

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

7367
describe('test cleanNodes/getDirtyNodes', function() {
74-
var engine;
7568

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

8982
it('should return all dirty nodes', function() {
90-
var nodes = engine.getDirtyNodes();
91-
83+
let nodes = engine.getDirtyNodes();
9284
expect(nodes.length).toEqual(2);
9385
expect(nodes[0].idx).toEqual(1);
9486
expect(nodes[1].idx).toEqual(2);
@@ -97,20 +89,16 @@ describe('gridstack engine', function() {
9789
it('should\'n clean nodes if _batchMode true', function() {
9890
engine._batchMode = true;
9991
engine.cleanNodes();
100-
10192
expect(engine.getDirtyNodes().length).toBeGreaterThan(0);
10293
});
10394

10495
it('should clean all dirty nodes', function() {
10596
engine.cleanNodes();
106-
10797
expect(engine.getDirtyNodes().length).toEqual(0);
10898
});
10999
});
110100

111101
describe('test batchUpdate/commit', function() {
112-
var engine;
113-
114102
beforeAll(function() {
115103
engine = new GridStack.Engine(12);
116104
});
@@ -137,7 +125,6 @@ describe('gridstack engine', function() {
137125
});
138126

139127
describe('test batchUpdate/commit', function() {
140-
var engine;
141128

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

157144
describe('test _notify', function() {
158-
var engine;
159-
var spy;
145+
let spy;
160146

161147
beforeEach(function() {
162148
spy = {
163149
callback: function() {}
164150
};
165151
spyOn(spy, 'callback');
166-
167152
engine = new GridStack.Engine(12, spy.callback, true);
168-
169153
engine.nodes = [
170154
engine._prepareNode({x: 0, y: 0, width: 1, height: 1, idx: 1, _dirty: true}),
171155
engine._prepareNode({x: 3, y: 2, width: 3, height: 2, idx: 2, _dirty: true}),
@@ -176,24 +160,20 @@ describe('gridstack engine', function() {
176160
it('should\'n be called if _batchMode true', function() {
177161
engine._batchMode = true;
178162
engine._notify();
179-
180163
expect(spy.callback).not.toHaveBeenCalled();
181164
});
182165

183166
it('should by called with dirty nodes', function() {
184167
engine._notify();
185-
186168
expect(spy.callback).toHaveBeenCalledWith([
187169
engine.nodes[0],
188170
engine.nodes[1]
189171
], true);
190172
});
191173

192174
it('should by called with extra passed node to be removed', function() {
193-
var n1 = {idx: -1};
194-
175+
let n1 = {idx: -1};
195176
engine._notify(n1);
196-
197177
expect(spy.callback).toHaveBeenCalledWith([
198178
n1,
199179
engine.nodes[0],
@@ -202,10 +182,8 @@ describe('gridstack engine', function() {
202182
});
203183

204184
it('should by called with extra passed node to be removed and should maintain false parameter', function() {
205-
var n1 = {idx: -1};
206-
185+
let n1 = {idx: -1};
207186
engine._notify(n1, false);
208-
209187
expect(spy.callback).toHaveBeenCalledWith([
210188
n1,
211189
engine.nodes[0],
@@ -216,12 +194,6 @@ describe('gridstack engine', function() {
216194

217195
describe('test _packNodes', function() {
218196
describe('using not float mode', function() {
219-
var engine;
220-
221-
var findNode = function(engine, id) {
222-
return engine.nodes.find(function(i) { return i._id === id; });
223-
};
224-
225197
beforeEach(function() {
226198
engine = new GridStack.Engine(12, null, false);
227199
});
@@ -230,9 +202,7 @@ describe('gridstack engine', function() {
230202
engine.nodes = [
231203
{x: 0, y: 0, width: 1, height: 1, _id: 1},
232204
];
233-
234205
engine._packNodes();
235-
236206
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1}));
237207
expect(findNode(engine, 1)._dirty).toBeFalsy();
238208
});
@@ -241,9 +211,7 @@ describe('gridstack engine', function() {
241211
engine.nodes = [
242212
{x: 0, y: 1, width: 1, height: 1, _id: 1},
243213
];
244-
245214
engine._packNodes();
246-
247215
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1, _dirty: true}));
248216
});
249217

@@ -252,33 +220,27 @@ describe('gridstack engine', function() {
252220
{x: 0, y: 1, width: 1, height: 1, _id: 1},
253221
{x: 0, y: 5, width: 1, height: 1, _id: 2},
254222
];
255-
256223
engine._packNodes();
257-
258224
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1, _dirty: true}));
259225
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 1, height: 1, _dirty: true}));
260226
});
261-
227+
262228
it('should pack nodes correctly', function() {
263229
engine.nodes = [
264230
{x: 0, y: 5, width: 1, height: 1, _id: 1},
265231
{x: 0, y: 1, width: 1, height: 1, _id: 2},
266232
];
267-
268233
engine._packNodes();
269-
270234
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 0, width: 1, height: 1, _dirty: true}));
271235
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 1, height: 1, _dirty: true}));
272236
});
273-
237+
274238
it('should respect locked nodes', function() {
275239
engine.nodes = [
276240
{x: 0, y: 1, width: 1, height: 1, _id: 1, locked: true},
277241
{x: 0, y: 5, width: 1, height: 1, _id: 2},
278242
];
279-
280243
engine._packNodes();
281-
282244
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 1, height: 1}));
283245
expect(findNode(engine, 1)._dirty).toBeFalsy();
284246
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 2, width: 1, height: 1, _dirty: true}));
@@ -287,35 +249,57 @@ describe('gridstack engine', function() {
287249
});
288250

289251
describe('test isNodeChangedPosition', function() {
290-
var engine;
291-
292252
beforeAll(function() {
293253
engine = new GridStack.Engine(12);
294254
});
295-
296255
it('should return true for changed x', function() {
297-
var widget = { x: 1, y: 2, width: 3, height: 4 };
256+
let widget = { x: 1, y: 2, width: 3, height: 4 };
298257
expect(engine.isNodeChangedPosition(widget, 2, 2)).toEqual(true);
299258
});
300-
301259
it('should return true for changed y', function() {
302-
var widget = { x: 1, y: 2, width: 3, height: 4 };
260+
let widget = { x: 1, y: 2, width: 3, height: 4 };
303261
expect(engine.isNodeChangedPosition(widget, 1, 1)).toEqual(true);
304262
});
305-
306263
it('should return true for changed width', function() {
307-
var widget = { x: 1, y: 2, width: 3, height: 4 };
264+
let widget = { x: 1, y: 2, width: 3, height: 4 };
308265
expect(engine.isNodeChangedPosition(widget, 2, 2, 4, 4)).toEqual(true);
309266
});
310-
311267
it('should return true for changed height', function() {
312-
var widget = { x: 1, y: 2, width: 3, height: 4 };
268+
let widget = { x: 1, y: 2, width: 3, height: 4 };
313269
expect(engine.isNodeChangedPosition(widget, 1, 2, 3, 3)).toEqual(true);
314270
});
315-
316271
it('should return false for unchanged position', function() {
317-
var widget = { x: 1, y: 2, width: 3, height: 4 };
272+
let widget = { x: 1, y: 2, width: 3, height: 4 };
318273
expect(engine.isNodeChangedPosition(widget, 1, 2, 3, 4)).toEqual(false);
319274
});
320275
});
276+
277+
describe('test locked widget', function() {
278+
beforeAll(function() {
279+
engine = new GridStack.Engine(12);
280+
});
281+
it('should add widgets around locked one', function() {
282+
let nodes = [
283+
{x: 0, y: 1, width: 12, height: 1, locked: 'yes', noMove: true, noResize: true, _id: 1},
284+
{x: 1, y: 0, width: 2, height: 3, _id: 2}
285+
];
286+
// add locked item
287+
engine.addNode(nodes[0])
288+
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 12, height: 1, locked: 'yes'}));
289+
engine.addNode(nodes[1])
290+
// add item that moves past locked one
291+
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, width: 12, height: 1, locked: 'yes'}));
292+
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 1, y: 2}));
293+
// prevents moving locked item
294+
let node1 = findNode(engine, 1);
295+
expect(engine.moveNode(node1, 6, 6)).toEqual(null);
296+
// but moves regular one (gravity ON)
297+
let node2 = findNode(engine, 2);
298+
expect(engine.moveNode(node2, 6, 6)).toEqual(jasmine.objectContaining({x: 6, y: 2, width: 2, height: 3,}));
299+
// but moves regular one (gravity OFF)
300+
engine.float = true;
301+
expect(engine.moveNode(node2, 7, 6)).toEqual(jasmine.objectContaining({x: 7, y: 6, width: 2, height: 3,}));
302+
});
303+
});
304+
321305
});

‎src/gridstack.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,15 @@
342342
while (true) {
343343
var collisionNode = this.nodes.find(Utils._collisionNodeCheck, {node: node, nn: nn});
344344
if (!collisionNode) { return; }
345-
var moved = this.moveNode(collisionNode, collisionNode.x, node.y + node.height,
346-
collisionNode.width, collisionNode.height, true);
345+
var moved;
346+
if (collisionNode.locked) {
347+
// if colliding with a locked item, move ourself instead
348+
moved = this.moveNode(node, node.x, collisionNode.y + collisionNode.height,
349+
node.width, node.height, true);
350+
} else {
351+
moved = this.moveNode(collisionNode, collisionNode.x, node.y + node.height,
352+
collisionNode.width, collisionNode.height, true);
353+
}
347354
if (!moved) { return; } // break inf loop if we couldn't move after all (ex: maxRow, fixed)
348355
}
349356
};
@@ -635,6 +642,7 @@
635642
};
636643

637644
GridStackEngine.prototype.moveNode = function(node, x, y, width, height, noPack) {
645+
if (node.locked) { return null; }
638646
if (typeof x !== 'number') { x = node.x; }
639647
if (typeof y !== 'number') { y = node.y; }
640648
if (typeof width !== 'number') { width = node.width; }

0 commit comments

Comments
 (0)