Skip to content

Commit 79ee8d8

Browse files
adumesnyadumesny
committed
calling load() allows overlapping widgets
* fix gridstack#2492 * make sure we use existing items and not reset to start from scratch so we do collision at each step.
1 parent 27735fe commit 79ee8d8

File tree

5 files changed

+51
-11
lines changed

5 files changed

+51
-11
lines changed

‎doc/CHANGES.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ Change log
55
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
66
**Table of Contents** *generated with [DocToc](http://doctoc.herokuapp.com/)*
77

8+
- [9.3.0-dev (TBD)](#930-dev-tbd)
89
- [9.3.0 (2023-09-30)](#930-2023-09-30)
910
- [9.2.2 (2023-09-27)](#922-2023-09-27)
1011
- [9.2.1 (2023-09-20)](#921-2023-09-20)
@@ -101,6 +102,9 @@ Change log
101102

102103
<!-- END doctoc generated TOC please keep comment here to allow auto update -->
103104

105+
## 9.3.0-dev (TBD)
106+
* fix [#2492](https://github.com/gridstack/gridstack.js/issues/2492) calling load() allows overlapping widgets
107+
104108
## 9.3.0 (2023-09-30)
105109
* fix [#1275](https://github.com/gridstack/gridstack.js/issues/1275) div scale support - Thank you [VincentMolinie](https://github.com/VincentMolinie) for implementing this
106110

‎spec/e2e/html/2492_load_twice.html

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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>load() twice bug</title>
8+
9+
<link rel="stylesheet" href="../../../demo/demo.css" />
10+
<script src="../../../dist/gridstack-all.js"></script>
11+
12+
</head>
13+
<body>
14+
<div class="container-fluid">
15+
<h1>load twice bug</h1>
16+
<p>this load() twice in a row with overlapping content and floating item</p>
17+
<div class="grid-stack"></div>
18+
</div>
19+
<script type="text/javascript">
20+
let items = [
21+
{x: 0, y: 0, w:2, content: '0 wide'},
22+
{x: 1, y: 0, content: '1 over'},
23+
{x: 2, y: 1, content: '2 float'},
24+
];
25+
let count = 0;
26+
items.forEach(n => n.id = String(count++)); // TEST loading with ids
27+
let grid = GridStack.init({disableOneColumnMode: true, cellHeight: 70, margin: 5}).load(items)
28+
items.forEach(n => n.content += '*')
29+
grid.load(items);
30+
</script>
31+
</body>
32+
</html>

‎src/gridstack-engine.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ export class GridStackEngine {
655655
public moveNode(node: GridStackNode, o: GridStackMoveOpts): boolean {
656656
if (!node || /*node.locked ||*/ !o) return false;
657657
let wasUndefinedPack: boolean;
658-
if (o.pack === undefined) {
658+
if (o.pack === undefined && !this.batchMode) {
659659
wasUndefinedPack = o.pack = true;
660660
}
661661

‎src/gridstack.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -657,10 +657,10 @@ export class GridStack {
657657
* @example
658658
* see http://gridstackjs.com/demo/serialization.html
659659
*/
660-
public load(layout: GridStackWidget[], addRemove: boolean | AddRemoveFcn = GridStack.addRemoveCB || true): GridStack {
660+
public load(items: GridStackWidget[], addRemove: boolean | AddRemoveFcn = GridStack.addRemoveCB || true): GridStack {
661661
// if passed list has coordinates, use them (insert from end to beginning for conflict resolution) else force widget same order
662-
const haveCoord = layout.some(w => w.x !== undefined || w.y !== undefined);
663-
let items = haveCoord ? Utils.sort(layout, -1, this._prevColumn || this.getColumn()) : layout;
662+
const haveCoord = items.some(w => w.x !== undefined || w.y !== undefined);
663+
if (haveCoord) items = Utils.sort(items, -1, this._prevColumn || this.getColumn());
664664
this._insertNotAppend = haveCoord; // if we create in reverse order...
665665

666666
// if we're loading a layout into for example 1 column (_prevColumn is set only when going to 1) and items don't fit, make sure to save
@@ -681,7 +681,8 @@ export class GridStack {
681681
if (addRemove) {
682682
let copyNodes = [...this.engine.nodes]; // don't loop through array you modify
683683
copyNodes.forEach(n => {
684-
let item = items.find(w => n.id === w.id);
684+
if (!n.id) return;
685+
let item = Utils.find(items, n.id);
685686
if (!item) {
686687
if (GridStack.addRemoveCB)
687688
GridStack.addRemoveCB(this.el, n, false, false);
@@ -691,19 +692,16 @@ export class GridStack {
691692
});
692693
}
693694

694-
// now add/update the widgets - starting with an empty list to reduce collision and add no-coord ones at next available spot
695-
let copyNodes = this.engine.nodes;
696-
this.engine.nodes = [];
695+
// now add/update the widgets
697696
items.forEach(w => {
698-
let item = (w.id !== undefined) ? copyNodes.find(n => n.id === w.id) : undefined;
697+
let item = Utils.find(this.engine.nodes, w.id);
699698
if (item) {
700699
// check if missing coord, in which case find next empty slot with new (or old if missing) sizes
701700
if (w.autoPosition || w.x === undefined || w.y === undefined) {
702701
w.w = w.w || item.w;
703702
w.h = w.h || item.h;
704703
this.engine.findEmptyPosition(w);
705704
}
706-
this.engine.nodes.push(item); // now back to current list...
707705
this.update(item.el, w);
708706
if (w.subGridOpts?.children) { // update any sub grid as well
709707
let sub = item.el.querySelector('.grid-stack') as GridHTMLElement;
@@ -1202,6 +1200,7 @@ export class GridStack {
12021200
if (!n) return;
12031201
let w = Utils.cloneDeep(opt); // make a copy we can modify in case they re-use it or multiple items
12041202
delete w.autoPosition;
1203+
delete w.id;
12051204

12061205
// move/resize widget if anything changed
12071206
let keys = ['x', 'y', 'w', 'h'];
@@ -1244,7 +1243,7 @@ export class GridStack {
12441243
Utils.sanitizeMinMax(n);
12451244

12461245
// finally move the widget
1247-
if (m) this.moveNode(n, m);
1246+
if (m !== undefined) this.moveNode(n, m);
12481247
if (changed) { // move will only update x,y,w,h so update the rest too
12491248
this._writeAttr(el, n);
12501249
}

‎src/utils.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,11 @@ export class Utils {
147147
return nodes.sort((b, a) => ((b.x ?? 1000) + (b.y ?? 1000) * column)-((a.x ?? 1000) + (a.y ?? 1000) * column));
148148
}
149149

150+
/** find an item by id */
151+
static find(nodes: GridStackNode[], id: string): GridStackNode | undefined {
152+
return id ? nodes.find(n => n.id === id) : undefined;
153+
}
154+
150155
/**
151156
* creates a style sheet with style id under given parent
152157
* @param id will set the 'gs-style-id' attribute to that id

0 commit comments

Comments
 (0)