Skip to content

much faster removeAll(), single event trigger #1097

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.

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
merged 2 commits into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion demo/responsive.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ <h1>Responsive grid demo</h1>
this.grid = $('.grid-stack').data('gridstack');

this.loadGrid = function () {
this.grid.batchUpdate();
this.grid.removeAll();
var items = GridStackUI.Utils.sort(this.serializedData);
this.grid.batchUpdate();
items.forEach(function (node, i) {
this.grid.addWidget($('<div><div class="grid-stack-item-content">' + i + '</div></div>'), node);
}.bind(this));
Expand Down
2 changes: 1 addition & 1 deletion demo/serialization.html
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ <h1>Serialization demo</h1>
this.grid = $('.grid-stack').data('gridstack');

this.loadGrid = function () {
this.grid.batchUpdate();
this.grid.removeAll();
var items = GridStackUI.Utils.sort(this.serializedData);
this.grid.batchUpdate();
items.forEach(function (node) {
this.grid.addWidget($('<div><div class="grid-stack-item-content"></div></div>'), node);
}, this);
Expand Down
1 change: 1 addition & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Change log
- fixed callbacks to get either `added, removed, change` or combination if adding a node require also to change its (x,y) for example.
Also you can now call `batchUpdate()` before calling a bunch of `addWidget()` and get a single event callback (more efficient).
[#1096](https://github.com/gridstack/gridstack.js/pull/1096)
- `removeAll()` is now much faster (no relayout) and calls `removed` event just once with a list

## v0.5.5 (2019-11-27)

Expand Down
2 changes: 1 addition & 1 deletion spec/e2e/html/gridstack-with-height.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ <h1>gridstack.js tests</h1>
];

this.grid = $('.grid-stack').data('gridstack');
this.grid.batchUpdate();
this.grid.removeAll();
items = GridStackUI.Utils.sort(items);
var id = 0;
this.grid.batchUpdate();
items.forEach(function(node) {
var w = $('<div><div class="grid-stack-item-content"></div></div>');
w.attr('id', 'item-' + (++id));
Expand Down
68 changes: 65 additions & 3 deletions spec/gridstack-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ describe('gridstack', function() {
var gridstackHTML =
'<div style="width: 992px; height: 800px" id="gs-cont">' +
' <div class="grid-stack">' +
' <div class="grid-stack-item" data-gs-x="0" data-gs-y="0" data-gs-width="4" data-gs-height="2">' +
' <div class="grid-stack-item" data-gs-x="0" data-gs-y="0" data-gs-width="4" data-gs-height="2" id="item1">' +
' <div class="grid-stack-item-content"></div>' +
' </div>' +
' <div class="grid-stack-item" data-gs-x="4" data-gs-y="0" data-gs-width="4" data-gs-height="4">' +
' <div class="grid-stack-item" data-gs-x="4" data-gs-y="0" data-gs-width="4" data-gs-height="4" id="item2">' +
' <div class="grid-stack-item-content"></div>' +
' </div>' +
' </div>' +
'</div>';
// generic widget with no param
var widgetHTML = '<div class="grid-stack-item"><div class="grid-stack-item-content"> hello </div></div>';
var widgetHTML = '<div class="grid-stack-item" id="item3"><div class="grid-stack-item-content"> hello </div></div>';

beforeEach(function() {
w = window;
Expand Down Expand Up @@ -457,6 +457,68 @@ describe('gridstack', function() {
});
});

describe('grid.removeAll', function() {
beforeEach(function() {
document.body.insertAdjacentHTML('afterbegin', gridstackHTML);
});
afterEach(function() {
document.body.removeChild(document.getElementById('gs-cont'));
});
it('should remove all children by default', function() {
$('.grid-stack').gridstack();
var grid = $('.grid-stack').data('gridstack');
grid.removeAll();
expect(grid.grid.nodes).toEqual([]);
expect(document.getElementById('item1')).toBe(null);
});
it('should remove all children', function() {
$('.grid-stack').gridstack();
var grid = $('.grid-stack').data('gridstack');
grid.removeAll(true);
expect(grid.grid.nodes).toEqual([]);
expect(document.getElementById('item1')).toBe(null);
});
it('should remove gridstack part, leave DOM behind', function() {
$('.grid-stack').gridstack();
var grid = $('.grid-stack').data('gridstack');
grid.removeAll(false);
expect(grid.grid.nodes).toEqual([]);
expect(document.getElementById('item1')).not.toBe(null);
});
});

describe('grid.removeWidget', function() {
beforeEach(function() {
document.body.insertAdjacentHTML('afterbegin', gridstackHTML);
});
afterEach(function() {
document.body.removeChild(document.getElementById('gs-cont'));
});
it('should remove first item (default), then second (true), then third (false)', function() {
$('.grid-stack').gridstack();
var grid = $('.grid-stack').data('gridstack');
expect(grid.grid.nodes.length).toEqual(2);

var el1 = document.getElementById('item1');
expect(el1).not.toBe(null);
grid.removeWidget(el1);
expect(grid.grid.nodes.length).toEqual(1);
expect(document.getElementById('item1')).toBe(null);
expect(document.getElementById('item2')).not.toBe(null);

var el2 = document.getElementById('item2');
grid.removeWidget(el2, true);
expect(grid.grid.nodes.length).toEqual(0);
expect(document.getElementById('item2')).toBe(null);

var el3 = grid.addWidget(widgetHTML);
expect(el3).not.toBe(null);
grid.removeWidget(el3, false);
expect(grid.grid.nodes.length).toEqual(0);
expect(document.getElementById('item3')).not.toBe(null);
});
});

describe('grid method obsolete warnings', function() {
beforeEach(function() {
document.body.insertAdjacentHTML('afterbegin', gridstackHTML);
Expand Down
38 changes: 21 additions & 17 deletions src/gridstack.js
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,8 @@
GridStackEngine.prototype._notify = function() {
if (this._batchMode) { return; }
var args = Array.prototype.slice.call(arguments, 0);
args[0] = args[0] === undefined ? [] : [args[0]];
args[1] = args[1] === undefined ? true : args[1];
args[0] = (args[0] === undefined ? [] : (Array.isArray(args[0]) ? args[0] : [args[0]]) );
args[1] = (args[1] === undefined ? true : args[1]);
var dirtyNodes = args[0].concat(this.getDirtyNodes());
this.onchange(dirtyNodes, args[1]);
};
Expand Down Expand Up @@ -516,14 +516,23 @@
};

GridStackEngine.prototype.removeNode = function(node, detachNode) {
detachNode = detachNode === undefined ? true : detachNode;
this._removedNodes.push(Utils.clone(node));
detachNode = (detachNode === undefined ? true : detachNode);
this._removedNodes.push(node);
node._id = null;
this.nodes = Utils.without(this.nodes, node);
this._packNodes();
this._notify(node, detachNode);
};

GridStackEngine.prototype.removeAll = function(detachNode) {
if (this.nodes.length === 0) { return; }
detachNode = (detachNode === undefined ? true : detachNode);
this.nodes.forEach(function(n) { n._id = null; }); // hint that node is being removed
this._removedNodes = this.nodes;
this.nodes = [];
this._notify(this._removedNodes, detachNode);
};

GridStackEngine.prototype.canMoveNode = function(node, x, y, width, height) {
if (!this.isNodeChangedPosition(node, x, y, width, height)) {
return false;
Expand Down Expand Up @@ -764,7 +773,7 @@
this._initStyles();

this.grid = new GridStackEngine(this.opts.column, function(nodes, detachNode) {
detachNode = detachNode === undefined ? true : detachNode;
detachNode = (detachNode === undefined ? true : detachNode);
var maxHeight = 0;
this.nodes.forEach(function(n) {
maxHeight = Math.max(maxHeight, n.y + n.height);
Expand Down Expand Up @@ -1475,31 +1484,27 @@
};

GridStack.prototype.removeWidget = function(el, detachNode) {
detachNode = detachNode === undefined ? true : detachNode;
detachNode = (detachNode === undefined ? true : detachNode);
el = $(el);
var node = el.data('_gridstack_node');

// For Meteor support: https://github.com/gridstack/gridstack.js/pull/272
if (!node) {
node = this.grid.getNodeDataByDOMEl(el);
}

this.grid.removeNode(node, detachNode);
el.removeData('_gridstack_node');
this._updateContainerHeight();
if (detachNode) {
el.remove();
}
this.grid.removeNode(node, detachNode);
this._triggerRemoveEvent();
// this._triggerChangeEvent(true); already have removeEvent
};

GridStack.prototype.removeAll = function(detachNode) {
// remove our data structure before list gets emptied (in case detachNode is false)
this.grid.nodes.forEach(function(node) {
this.removeWidget(node.el, detachNode);
}, this);
this.grid.nodes = [];
this._updateContainerHeight();
node.el.removeData('_gridstack_node');
});
this.grid.removeAll(detachNode);
this._triggerRemoveEvent();
};

GridStack.prototype.destroy = function(detachGrid) {
Expand Down Expand Up @@ -1761,7 +1766,6 @@

GridStack.prototype.commit = function() {
this.grid.commit();
this._updateContainerHeight();
this._triggerRemoveEvent();
this._triggerAddEvent();
this._triggerChangeEvent();
Expand Down