Skip to content

Commit f1af512

Browse files
author
Alain Dumesny
committed
maxRow now limit initial item placement
* it was possible before to `addWidget()` outside of a maxRow limit, causing the item to not be draggable * had to be careful not to break knockout demos * fix for #1155
1 parent c0e6297 commit f1af512

File tree

3 files changed

+45
-50
lines changed

3 files changed

+45
-50
lines changed

‎doc/CHANGES.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ Change log
3333

3434
## 1.0.0-dev (upcoming)
3535

36-
-- fix [(1166)](https://github.com/gridstack/gridstack.js/issues/1166) resize not taking margin height into account
36+
- fix [(1166)](https://github.com/gridstack/gridstack.js/issues/1166) resize not taking margin height into account
37+
- fix [(1155)](https://github.com/gridstack/gridstack.js/issues/1155) `maxRow` now limit initial item placement if out of bound, preventing broken drag behavior
3738

3839
## v1.0.0 (2020-02-23)
3940

‎spec/gridstack-spec.js

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ describe('gridstack', function() {
828828
expect(parseInt($widget.attr('data-gs-y'), 10)).toBe(0);
829829
expect(parseInt($widget.attr('data-gs-width'), 10)).toBe(1);
830830
expect(parseInt($widget.attr('data-gs-height'), 10)).toBe(2);
831-
expect($widget.attr('data-gs-auto-position')).toBe(undefined);
831+
expect($widget.attr('data-gs-auto-position')).toBe('true');
832832
expect($widget.attr('data-gs-min-width')).toBe(undefined);
833833
expect($widget.attr('data-gs-max-width')).toBe(undefined);
834834
expect($widget.attr('data-gs-min-height')).toBe(undefined);
@@ -843,7 +843,7 @@ describe('gridstack', function() {
843843
expect(parseInt($widget.attr('data-gs-y'), 10)).toBe(0);
844844
expect(parseInt($widget.attr('data-gs-width'), 10)).toBe(1);
845845
expect(parseInt($widget.attr('data-gs-height'), 10)).toBe(2);
846-
expect($widget.attr('data-gs-auto-position')).toBe(undefined);
846+
expect($widget.attr('data-gs-auto-position')).toBe('true');
847847
expect($widget.attr('data-gs-min-width')).toBe(undefined);
848848
expect($widget.attr('data-gs-max-width')).toBe(undefined);
849849
expect($widget.attr('data-gs-min-height')).toBe(undefined);
@@ -858,7 +858,7 @@ describe('gridstack', function() {
858858
expect(parseInt($widget.attr('data-gs-y'), 10)).toBe(0);
859859
expect(parseInt($widget.attr('data-gs-width'), 10)).toBe(1);
860860
expect(parseInt($widget.attr('data-gs-height'), 10)).toBe(2);
861-
expect($widget.attr('data-gs-auto-position')).toBe(undefined);
861+
expect($widget.attr('data-gs-auto-position')).toBe('true');
862862
expect($widget.attr('data-gs-min-width')).toBe(undefined);
863863
expect($widget.attr('data-gs-max-width')).toBe(undefined);
864864
expect($widget.attr('data-gs-min-height')).toBe(undefined);
@@ -873,7 +873,7 @@ describe('gridstack', function() {
873873
expect(parseInt($widget.attr('data-gs-y'), 10)).toBe(0);
874874
expect(parseInt($widget.attr('data-gs-width'), 10)).toBe(1);
875875
expect(parseInt($widget.attr('data-gs-height'), 10)).toBe(2);
876-
expect($widget.attr('data-gs-auto-position')).toBe(undefined);
876+
expect($widget.attr('data-gs-auto-position')).toBe('true');
877877
expect($widget.attr('data-gs-min-width')).toBe(undefined);
878878
expect($widget.attr('data-gs-max-width')).toBe(undefined);
879879
expect($widget.attr('data-gs-min-height')).toBe(undefined);
@@ -888,7 +888,7 @@ describe('gridstack', function() {
888888
expect(parseInt($widget.attr('data-gs-y'), 10)).toBe(0);
889889
expect(parseInt($widget.attr('data-gs-width'), 10)).toBe(1);
890890
expect(parseInt($widget.attr('data-gs-height'), 10)).toBe(1);
891-
expect($widget.attr('data-gs-auto-position')).toBe(undefined);
891+
expect($widget.attr('data-gs-auto-position')).toBe('true');
892892
expect($widget.attr('data-gs-min-width')).toBe(undefined);
893893
expect($widget.attr('data-gs-max-width')).toBe(undefined);
894894
expect($widget.attr('data-gs-min-height')).toBe(undefined);
@@ -1032,24 +1032,16 @@ describe('gridstack', function() {
10321032
afterEach(function() {
10331033
document.body.removeChild(document.getElementById('gs-cont'));
10341034
});
1035-
it('should do nothing and return node', function() {
1036-
var options = {
1037-
cellHeight: 80,
1038-
verticalMargin: 10
1039-
};
1040-
var grid = GridStack.init(options);
1035+
it('should do nothing and return NULL to mean nothing happened', function() {
1036+
var grid = GridStack.init();
10411037
var items = $('.grid-stack-item');
10421038
grid._updateElement(items[0], function(el, node) {
1043-
var newNode = grid.engine.moveNode(node);
1044-
expect(newNode).toBe(node);
1039+
var hasMoved = grid.engine.moveNode(node);
1040+
expect(hasMoved).toBe(null);
10451041
});
10461042
});
10471043
it('should do nothing and return node', function() {
1048-
var options = {
1049-
cellHeight: 80,
1050-
verticalMargin: 10
1051-
};
1052-
var grid = GridStack.init(options);
1044+
var grid = GridStack.init();
10531045
var items = $('.grid-stack-item');
10541046
grid.minWidth(items[0], 1);
10551047
grid.maxWidth(items[0], 2);

‎src/gridstack.js

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,9 @@
342342
while (true) {
343343
var collisionNode = this.nodes.find(Utils._collisionNodeCheck, {node: node, nn: nn});
344344
if (!collisionNode) { return; }
345-
this.moveNode(collisionNode, collisionNode.x, node.y + node.height,
345+
var moved = this.moveNode(collisionNode, collisionNode.x, node.y + node.height,
346346
collisionNode.width, collisionNode.height, true);
347+
if (!moved) { return; } // break inf loop if we couldn't move after all (ex: maxRow, fixed)
347348
}
348349
};
349350

@@ -432,19 +433,28 @@
432433
if (Number.isNaN(node.width)) { node.width = defaults.width; }
433434
if (Number.isNaN(node.height)) { node.height = defaults.height; }
434435

436+
if (node.maxWidth !== undefined) { node.width = Math.min(node.width, node.maxWidth); }
437+
if (node.maxHeight !== undefined) { node.height = Math.min(node.height, node.maxHeight); }
438+
if (node.minWidth !== undefined) { node.width = Math.max(node.width, node.minWidth); }
439+
if (node.minHeight !== undefined) { node.height = Math.max(node.height, node.minHeight); }
440+
435441
if (node.width > this.column) {
436442
node.width = this.column;
437443
} else if (node.width < 1) {
438444
node.width = 1;
439445
}
440-
441-
if (node.height < 1) {
446+
if (this.maxRow && node.height > this.maxRow) {
447+
node.height = this.maxRow;
448+
} else if (node.height < 1) {
442449
node.height = 1;
443450
}
444451

445452
if (node.x < 0) {
446453
node.x = 0;
447454
}
455+
if (node.y < 0) {
456+
node.y = 0;
457+
}
448458

449459
if (node.x + node.width > this.column) {
450460
if (resizing) {
@@ -453,9 +463,12 @@
453463
node.x = this.column - node.width;
454464
}
455465
}
456-
457-
if (node.y < 0) {
458-
node.y = 0;
466+
if (this.maxRow && node.y + node.height > this.maxRow) {
467+
if (resizing) {
468+
node.height = this.maxRow - node.y;
469+
} else {
470+
node.y = this.maxRow - node.height;
471+
}
459472
}
460473

461474
return node;
@@ -497,11 +510,6 @@
497510
GridStackEngine.prototype.addNode = function(node, triggerAddEvent) {
498511
node = this._prepareNode(node);
499512

500-
if (node.maxWidth !== undefined) { node.width = Math.min(node.width, node.maxWidth); }
501-
if (node.maxHeight !== undefined) { node.height = Math.min(node.height, node.maxHeight); }
502-
if (node.minWidth !== undefined) { node.width = Math.max(node.width, node.minWidth); }
503-
if (node.minHeight !== undefined) { node.height = Math.max(node.height, node.minHeight); }
504-
505513
node._id = node._id || ++idSeq;
506514

507515
if (node.autoPosition) {
@@ -632,29 +640,21 @@
632640
if (typeof width !== 'number') { width = node.width; }
633641
if (typeof height !== 'number') { height = node.height; }
634642

635-
if (node.maxWidth !== undefined) { width = Math.min(width, node.maxWidth); }
636-
if (node.maxHeight !== undefined) { height = Math.min(height, node.maxHeight); }
637-
if (node.minWidth !== undefined) { width = Math.max(width, node.minWidth); }
638-
if (node.minHeight !== undefined) { height = Math.max(height, node.minHeight); }
639-
640-
if (node.x === x && node.y === y && node.width === width && node.height === height) {
641-
return node;
643+
// constrain the passed in values and check if we're still changing our node
644+
var resizing = (node.width !== width || node.height !== height);
645+
var nn = { x: x, y: y, width: width, height: height,
646+
maxWidth: node.maxWidth, maxHeight: NodeIterator.maxHeight, minWidth: node.minWidth, minHeight: node.minHeight};
647+
nn = this._prepareNode(nn, resizing);
648+
if (node.x === nn.x && node.y === nn.y && node.width === nn.width && node.height === nn.height) {
649+
return null;
642650
}
643651

644-
var resizing = node.width !== width;
645652
node._dirty = true;
646653

647-
node.x = x;
648-
node.y = y;
649-
node.width = width;
650-
node.height = height;
651-
652-
node.lastTriedX = x;
653-
node.lastTriedY = y;
654-
node.lastTriedWidth = width;
655-
node.lastTriedHeight = height;
656-
657-
node = this._prepareNode(node, resizing);
654+
node.x = node.lastTriedX = nn.x;
655+
node.y = node.lastTriedY = nn.y;
656+
node.width = node.lastTriedWidth = nn.width;
657+
node.height = node.lastTriedHeight = nn.height;
658658

659659
this._fixCollisions(node);
660660
if (!noPack) {
@@ -1462,14 +1462,16 @@
14621462
// Tempting to initialize the passed in opt with default and valid values, but this break knockout demos
14631463
// as the actual value are filled in when _prepareElement() calls el.attr('data-gs-xyz) before adding the node.
14641464
// opt = this.engine._prepareNode(opt);
1465-
opt = opt || {};
14661465
} else {
14671466
// old legacy way of calling with items spelled out - call us back with single object instead (so we can properly initialized values)
14681467
return this.addWidget(el, {x: opt, y: y, width: width, height: height, autoPosition: autoPosition,
14691468
minWidth: minWidth, maxWidth: maxWidth, minHeight: minHeight, maxHeight: maxHeight, id: id});
14701469
}
14711470

14721471
el = $(el);
1472+
if (opt) { // see knockout above
1473+
this.engine._prepareNode(opt);
1474+
}
14731475
this._writeAttr(el, opt);
14741476
this.$el.append(el);
14751477
return this.makeWidget(el);

0 commit comments

Comments
 (0)